1909028 - Firefox differs from Chromium/WebKit on whether xlink:href attr remains set, after it was modified and then 'href' attribute is removed

Status: NEW

(Core :: SVG, task)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (lime in Firefox, red in Chromium/WebKit)

I'm pretty sure this is a Chromium/WebKit bug and Firefox is correct, but I'm starting this out with a Mozilla bug report to have a place to post testcases and poke/discuss/document a little bit.

STR:

1. Load attached testcase (directly as a SVG document -- be sure you're not just letting Bugzilla present it as an image. It's important that the script runs.)
2. Look at the final rendering and at console.log.

FIREFOX RESULTS:
Firefox shows a lime rect, and logs "PASS" and a final xlink:href value of null.

CHROMIUM/WEBKIT RESULTS:
Chromium/WebKit both shows a red image, and log FAIL and a final xlink:href value with the red data-URI that gets set in the script.

There are several interesting modifications that I can make to the testcase that make Chromium/WebKit match Firefox, which leads me to believe that our behavior is probably correct. I'll post those modified versions as separate testcases.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (removed initial xlink:href value, which makes WebKit/Chromium match Firefox)

If I don't include a xlink:href="" attribute on the image in the raw SVG, then Chromium and WebKit change to match Firefox's behavior as described in comment 0. Firefox keeps the same behavior.

(The presence/absence of this attribute shouldn't make a difference, since this initial xlink:href attr-value gets overwritten by the setAttributeNS expression in the script anyway. So Firefox's behavior -- rendering testcase 1 the same as the consensus rendering of this testcase -- seems reasonable.)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testecase 3 (removed dynamic xlink:href mutation, which makes Firefox match WebKit/Chromium)

On the other hand, if I remove this attribute-modification...

    image.setAttributeNS(XLINK_NS, 'href', RED_DATA_URI);

(and instead just fold the RED_DATA_URI to be the initial static value of xlink:href to make the attribute's presence/absence still clear), then Firefox changes to match Chrome and WebKit in preserving the xlink:href value despite the href attribute being removed.

Comment 3

[Daniel Holbert [:dholbert]]

So: things are a bit weird here.

Summing up what the testcases demonstrate:

testcase 1: If there's a xlink:href attribute-value up-front and it gets dynamically modified (with setAttributeNS), and then we do image.removeAttribute('href') to remove the no-namespace href attribute (which was never present), then Firefox actually removes the xlink:href attribute, while other browsers do not remove it.

testcase 2: If we do the same stuff in testcase 1 but xlink:href is not set statically up-front (but still gets set via script), then all browsers agree that the no-namespace-attribute-removal image.removeAttribute('href') should actually remove the xlink:href attribute.

testcase 3: If we do the same stuff in testcase 1 but xlink:href is *not modified via script (but still set statically up-front), then all browsers agree that the no-namespace-attribute-removal image.removeAttribute('href') should NOT remove the xlink:href attribute.

Bottom line, it seems like Firefox is internally consistent when comparing testcases 1 and 2, whereas other browsers are internally consistent when comparing testcases 1 and 3.

Comment 4

[Robert Longson [:longsonr]]

Regarding testcase 1 (and testcase 2).

Element::RemoveAttribute calls Element::InternalGetAttrNameFromQName that matches on name only and ignores namespaces. It returns the xlink:href attribute and then we continue as if we're trying to delete the xlink:href attribute rather than the href attribute. i.e. name->NamespaceID() is the xlink namespace because we found a href attribute in the xlink namespace.

Is that how it's supposed to work? It seems pretty odd that we'd remove the first thing we find regardless of its actual namespace. If there are multiple href attributes in different namespaces then we'd remove one of them wouldn't we. I'm not sure which one.

Regarding testcase 3 again we seem to go through the same path so I'm not sure why we're not removing the xlink:href attribute.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

After image.setAttributeNS(XLINK_NS, 'href', RED_DATA_URI); call, the qualified name is "href", not xlink:href, so image.removeAttribute('href'); can remove that,
since that finds the attribute per qualified name https://dom.spec.whatwg.org/#concept-element-attributes-remove-by-name

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

oh, actually, that setAttributeNS shouldn't change qname
https://dom.spec.whatwg.org/#concept-element-attributes-set-value
https://searchfox.org/mozilla-central/rev/b3c85ac11d004fdb582577cd8f674efa44b0e253/dom/base/Element.cpp#2681-2684 could be tweaked a bit, I guess