sycamore icon indicating copy to clipboard operation
sycamore copied to clipboard

SVG attribute issues in version 0.7.1

Open Ben-PH opened this issue 2 years ago • 2 comments

Describe the bug setting an attribute on a node (in my case specifically: an svg node) using the builder interface results in a lower-cased name

To Reproduce Inside a component renderer:

svg().attr("viewBox", "...").build()

Expected behavior the resulting dom-node to be like so:

<svg viewBox="..."></svg>

instead I get:

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

Environment

sycamore version 0.7.1 with experimental builder feature

Additional context

One possibility is the use of the intern function in web-sys. It's the only place I can see that I could find that could clobber the string. I can't find anywhere else in our codebase where we are using uppercase attribute names, but with the particular behavior of svg nodes, and the ubiquity of the use of intern, I would be surprised if it was this.

this discord message suggests it's fixed in 0.8: https://discord.com/channels/820400041332179004/821489793258487839/981218246441197608

In discussion with colleagues, someone referred to this SO thread, which seems to be addressed in the 0.8 fix: https://stackoverflow.com/questions/28734628/how-can-i-set-an-attribute-with-case-sensitive-name-in-a-javascript-generated-el/28734954#28734954

Ben-PH avatar Jun 01 '22 09:06 Ben-PH

In discussion with colleagues, someone referred to this SO thread, which seems to be addressed in the 0.8 fix:

It is the change from #389 and your colleague is correct, it is because of this part of the code: https://github.com/sycamore-rs/sycamore/blob/5f58fe37599e125fdc4a85cbd51e4e1c3d359791/packages/sycamore-web/src/dom_node.rs#L153-L163

The WHATWG spec explains why the casing was changing, without the above, in the setAttribute section:

2. If this is in the HTML namespace and its node document is an HTML document, then set qualifiedName (name of attribute) to qualifiedName in ASCII lowercase.

If you want to see this in action then you can do so in the dev tools by adding a plain svg using create_element on the body then setAttribute with "viewBox" and whatever value - you will notice the case change in the element inspector. However, if you use setAttributeNS("http://www.w3.org/2000/svg", "viewBox", "..") it would work as the former would had if the svg was created with create_elementNS with "http://www.w3.org/2000/svg".

The intern function is just for caching and you can see from the source it is relatively simple and there is no altering of the &str :)

Hope all that helps :)

mc1098 avatar Jun 01 '22 15:06 mc1098

@lukechu10 with the context of 0.8 happening, consider this issue a polite/lightly-put request for a backport of the fix. lightly-put because it will be redundant quite soon, and (for us at least), is only a minor issue now that we have a workaround-hack that is ergonomic.

If a backport of the fix is not planned, it might be a good idea to add a todo item to deprecate the svg element in 0.7 with the message referencing this issue, and that it's fixed in 0.8

Ben-PH avatar Jun 02 '22 18:06 Ben-PH

Since v0.8 is released now, I shall be closing this issue.

lukechu10 avatar Sep 20 '22 20:09 lukechu10