A way for attributes to contain expressions without triggering browser errors or false loading

[DmitrySharabin]

Closes #475.

I believe this feature is a nice to have in the upcoming release. I tested it on several examples, including SVG Path Builder. It looks like it's working. :)

I liked the idea of using the mv-attr-* family of attributes for the feature, so I went that way.

Since I'm not 100% sure that I do everything correctly, let's consider this PR as the first step. :)

[DmitrySharabin]

Here is a couple of examples: https://codepen.io/dmitrysharabin/pen/gOoqOzj

[DmitrySharabin]

There is an issue I don't know how to address yet: I can't preserve an attribute case. E.g., mv-attr-veiwBox is converted to mv-attr-viewbox. And it doesn't let us use the mv-attr-* family of attributes with a bunch of SVG attributes.

I tried to add different attributes with letters in different cases to elements inside and outside a Mavo app, and it looks like browsers convert attributes to lower case automatically? Am I right?

@LeaVerou Are you aware of any way to bypass it?

[DmitrySharabin]

I tried to add different attributes with letters in different cases to elements inside and outside a Mavo app, and it looks like browsers convert attributes to lower case automatically? Am I right?

It looks like I am. 😭 https://developer.mozilla.org/en-US/docs/Web/API/Element/getAttribute#lower_casing

[karger]

awkward hack: define another attribute that converts its indicated attribute from hyphenated to camel case; e.g. mv-camelcase-view-box would become viewBox.

On 4/19/2022 9:08 AM, Dmitry Sharabin wrote:

I tried to add different attributes with letters in different
cases to elements inside and outside a Mavo app, and it looks like
browsers convert attributes to lower case automatically? Am I right?

It looks like I am. 😭 https://developer.mozilla.org/en-US/docs/Web/API/Element/getAttribute#lower_casing

— Reply to this email directly, view it on GitHub https://github.com/mavoweb/mavo/pull/833#issuecomment-1102631456, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIWSXUMRDHEZAGOSNIW2XDVF2V5XANCNFSM5TWYYJYQ. You are receiving this because you are subscribed to this thread.Message ID: @.***>

[karger]

actually, here is another possible approach: apparently there is a specified list of tag adjustments that you could perform automatically if the node with the attributes is an svg node which I suspect can be checked somehow.

https://html.spec.whatwg.org/multipage/parsing.html#adjust-svg-attributes

search for "adjust svg attributes" in this doc if the anchor fails.

On 4/19/2022 9:08 AM, Dmitry Sharabin wrote:

I tried to add different attributes with letters in different
cases to elements inside and outside a Mavo app, and it looks like
browsers convert attributes to lower case automatically? Am I right?

It looks like I am. 😭 https://developer.mozilla.org/en-US/docs/Web/API/Element/getAttribute#lower_casing

— Reply to this email directly, view it on GitHub https://github.com/mavoweb/mavo/pull/833#issuecomment-1102631456, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIWSXUMRDHEZAGOSNIW2XDVF2V5XANCNFSM5TWYYJYQ. You are receiving this because you are subscribed to this thread.Message ID: @.***>

[LeaVerou]

Oh I'm glad you're working on this. Is the casing issue cosmetic or does it actually cause loss of functionality?

Due to the way Mavo resolves expressions, substituting gradually, I imagine (without having seen the code) that this solution may still produce errors when the expressions depend on other expressions, e.g.:

<input type=number value="[foo]">
<span property="foo" value="[bar]"></span>
<span property="bar" value="3"></span>

[DmitrySharabin]

Is the casing issue cosmetic or does it actually cause loss of functionality?

In the SVG Path Builder we won't be able to get rid of the error caused by this code: viewBox="0 0 [width] [height]".

Due to the way Mavo resolves expressions, substituting gradually, I imagine (without having seen the code) that this solution may still produce errors when the expressions depend on other expressions...

Do you mean trigger browser errors? The following code works without errors (I updated the pen). :) But I'm almost 100% sure I could miss some use cases.

<input type=number mv-attr-value="[foo]" />
<span property="foo" mv-value="bar"></span>
<span property="bar">3</span>

[DmitrySharabin]

actually, here is another possible approach: apparently there is a specified list of tag adjustments that you could perform automatically if the node with the attributes is an svg node which I suspect can be checked somehow.

That's interesting. Thanks, David. 🙏🏻

I was thinking of the approach you suggested and wondering: do we really need this at the core? If the casing issue becomes a problem for authors, we can add a plugin that will make the mv-attr-* family of attributes case-sensitive. What do you think?

[LeaVerou]

Is the casing issue cosmetic or does it actually cause loss of functionality?

In the SVG Path Builder we won't be able to get rid of the error caused by this code: viewBox="0 0 [width] [height]".

Why not? Can't we specify mv-attr-viewBox and then your code would specify viewbox (lowercase), which I think would still work?

[DmitrySharabin]

Why not? Can't we specify mv-attr-viewBox and then your code would specify viewbox (lowercase), which I think would still work?

Ah, yes. You are absolutely right—lowercase attributes are not even an issue. They work perfectly fine. Stupid me. 🤦‍♂️

Now I need to figure out why viewBox, for example, is not live if it's set via mv-attr-viewbox. That's why I thought attributes case is an issue.

Thank you. I'll continue my investigation then.

[DmitrySharabin]

Thanks to David, we know that browsers adjust SVG attributes that are not all lowercase. That means that when there is an expression inside the value of an attribute (e.g., inside viewBox), Mavo gets the case-sensitive version of the attribute name to work with (e.g., viewBox). As a result, viewBox="0 0 [width] [height]" works perfectly fine (the view box adjusts according to the values of width and height).

The mv-attr-* attribute, on the other hand, is not the standard SVG attribute. So, for the mv-attr-viewBox Mavo will get the mv-attr-viewbox (all lowercase) to work with. And even though SVG understands viewbox="0 0 [width] [height]" (when added directly to <svg>), if this attribute is added through mv-attr-viewbox="0 0 [width] [height]", the expressions don't have any effect on the SVG. Though, the viewbox attribute is being updated via expressions. The same thing happens with preserveAspectRatio.

We can conclude that the casing issue causes loss of functionality. So I decided to follow David's suggestion and adjust those kinds of attributes manually. And it worked. Yay!

I updated the demo so that it showcases different use cases. UPDATE: Refactored SVG Path builder—no console errors! 🥳

I believe this PR is ready for review. 🤞🏻

[LeaVerou]

Are you using the correct namespace for these attributes? I wonder if that's why viewbox doesn't work.

[DmitrySharabin]

Are you using the correct namespace for these attributes? I wonder if that's why viewbox doesn't work.

Hmm. That's a good question. I don't know. I definitely need to figure it out.

[DmitrySharabin]

Here are some results of my research concerning SVG, its elements, attributes, and namespaces:

1. Regardless of whether we use viewBox or viewbox, the browser adjusts it to viewBox (and adds the corresponding attribute if needed).

Suppose we have (mind the attribute case):

<svg viewbox="0 0 100 100">
  ...
</svg>

In DevTools, we see (mind the attribute case):

After inspecting the <svg> element and invoking console.dir($0) in the console, we get:

2. Attributes of SVG elements don't have namespaceURI (even though the corresponding elements do). Except href, IIRC. See the previous screenshot.

3. Regardless the point 2 (it didn't stop me 😂), I searched through the codebase and found the method where we set attributes: Primitive.setValue(). There, I replaced this line

element.setAttribute(o.attribute, value);

with this one:

if (element.namespaceURI === "http://www.w3.org/2000/svg") {
  element.setAttributeNS(element.namespaceURI, o.attribute, value);
}
else {
  element.setAttribute(o.attribute, value);
}

Then I inspected <svg> added by the following code (mind the fallback for viewBox and its case):

<svg
  viewbox="-50 -50 50 50"
  mv-attr-viewBox="[list(0, 0, 100, 100)]">
  ...
</svg>

In DevTools we have

and

As we can see, the browser adjusted the fallback attribute (as expected), and the one added by Mavo has the namespaceURI property and is evaluated correctly (its value is the result of an expression evaluation), though it doesn't have any effect. It's simply ignored by the browser. Moreover, every other mv-attr-* attribute stopped working (even though without namespace they worked fine).

To conclude, namespaceURI doesn't have any positive effect in our case.

So, unfortunately I couldn't find the way to make mv-attr-* work with case-sensitive SVG attributes without manually tweaking them (as I do it in this PR). 😔 However, I can't deny that there might be something I still don't get. 🤷🏻‍♂️

[LeaVerou]

I believe namespaceURI is null when it can be determined via the parent, ancestor etc namespace. It's not null on href because href has the XLink namespace implicitly (in fact it used to be written xlink:href).

I wonder if something like this would work: https://codepen.io/leaverou/pen/ZEvPPaP?editors=1111 The problem is, you don't want to change casing if e.g. someone sets a viewbox attribute on an HTML element, so you can't just match on attribute names. Context matters.

[DmitrySharabin]

I wonder if something like this would work: https://codepen.io/leaverou/pen/ZEvPPaP?editors=1111

Whoa! That's cool!!! Thanks for the new knowledge.

Re:context. Got it. Totally agree. I will take that into account.

[DmitrySharabin]

Done!

[DmitrySharabin]

One complication is that not all attributes will work on the root element, some of them may need to be on specific elements. You should try and see if it works for e.g. refX and that sort of thing.

If I get your comment correctly, now it should work (after some fixes I made to the code): https://codepen.io/dmitrysharabin/pen/mdpgBvy

[DmitrySharabin]

@LeaVerou, I would also be grateful if you give this PR another look. I believe the feature it adds to Mavo might be very beneficial. We might make this feature experimental.

[DmitrySharabin]

Btw what happens if someone uses both mv-attr-foo and foo? Does the order matter?

As you mentioned in your original issue, “This has the added benefit of allowing fallbacks by just providing a value on the actual attribute.” So, yes, the order matters, and mv-attr-foo has precedence over foo as soon as Mavo steps into play. E.g,. this doesn't error anymore: <rect width="40" height="30" rx="3" ry="3" mv-attr-rx="[rx]" mv-attr-ry="[ry]" />. 🥳

[DmitrySharabin]

@LeaVerou, thank you so much for all your comments and suggestions. I took them into account. It would be nice if you could find time to review it again.

[LeaVerou]

Btw what happens if someone uses both mv-attr-foo and foo? Does the order matter?

As you mentioned in your original issue, “This has the added benefit of allowing fallbacks by just providing a value on the actual attribute.” So, yes, the order matters, and mv-attr-foo has precedence over foo as soon as Mavo steps into play. E.g,. this doesn't error anymore: <rect width="40" height="30" rx="3" ry="3" mv-attr-rx="[rx]" mv-attr-ry="[ry]" />. 🥳

Right, but what happens if the attribute also has an expression?

[DmitrySharabin]

Btw what happens if someone uses both mv-attr-foo and foo? Does the order matter?

As you mentioned in your original issue, “This has the added benefit of allowing fallbacks by just providing a value on the actual attribute.” So, yes, the order matters, and mv-attr-foo has precedence over foo as soon as Mavo steps into play. E.g,. this doesn't error anymore: <rect width="40" height="30" rx="3" ry="3" mv-attr-rx="[rx]" mv-attr-ry="[ry]" />. 🥳

Right, but what happens if the attribute also has an expression?

That's an excellent question. Hm. I may assume that if the user added mv-attr-foo, they expect it to have priority over foo. So, if we have both foo and mv-attr-foo, and foo has an expression, we should probably remove (unregister) it first and update foo with the expression we have on mv-attr-foo (the way we already do). Should we warn the user about the performed action if we follow this route?

I still might not see other possible issues you see. I'm open to all possible questions you might ask. And thank you for spending your time teaching me. I appreciate that! 🙏🏻

[LeaVerou]

I think as long as it's deterministic, there are many right choices. The main thing we should avoid is it being non-deterministic, i.e. sometimes it overrides it and sometimes it doesn't depending on race conditions.

I agree mv-attr-foo always having precedence over foo makes sense. I could also be persuaded that the order of attributes can specify precedence.

Thoughts, @karger @adamjanicki2 ?

[adamjanicki2]

I think as long as it's deterministic, there are many right choices. The main thing we should avoid is it being non-deterministic, i.e. sometimes it overrides it and sometimes it doesn't depending on race conditions.

Definitely on the same page here, a non-deterministic scenario would be very much not ideal 😂

I agree mv-attr-foo always having precedence over foo makes sense. I could also be persuaded that the order of attributes can specify precedence.

I think overall I like having mv-attr-attrname taking precedence or attrname, but also there is some precedence with ordering in other frameworks, where typically the last attr specified takes precedence (for example in react if you have something like <Element {...props} className="..." />), so I could also be persuaded for an ordering, but since Mavo has this specific feature for defining attributes, the goal should be encouraging users to use them over the default ones, which is why I like Mavo attrs taking precedence

Regardless of what we decide, we definitely will need to update the documentation to be very clear and explicit about how this works

[DmitrySharabin]

I think so! 🤞

[DmitrySharabin]

@LeaVerou, thanks a ton for your help! We made this awesome feature happen. Yay! 🥳

[karger]

I think as long as it's deterministic, there are many right choices. The main thing we should avoid is it being non-deterministic, i.e. sometimes it overrides it and sometimes it doesn't depending on race conditions.

I agree mv-attr-foo always having precedence over foo makes sense. I could also be persuaded that the order of attributes can specify precedence.

Thoughts, @karger @adamjanicki2 ?

I think the natural use case for foo= mv-attr-foo= is when you want to give the browser something to work with until the expression in mv-attr-foo is evaluated, but that once that happens mv-attr-foo should dominate.

[DmitrySharabin]

I think the natural use case for foo= mv-attr-foo= is when you want to give the browser something to work with until the expression in mv-attr-foo is evaluated, but that once that happens mv-attr-foo should dominate.

This is exactly how it’s designed and implemented. I’m glad we are all on the same page. ☺️

[karger]

I think the natural use case for foo= mv-attr-foo= is when you want to give the browser something to work with until the expression in mv-attr-foo is evaluated, but that once that happens mv-attr-foo should dominate.

This is exactly how it’s designed and implemented. I’m glad we are all on the same page. ☺️

Having considered further, I can see an argument for order of attributes, because that's how html works, right? So once the expression is evaluated. we could apply the standard order of attributes rules to decide if it overrides the foo attribute.