Bug 248747 – Serialize xmlns attributes first

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Ahmad Saleem]

Hi Team,

While going through Blink's commits:

URL - https://chromium.googlesource.com/chromium/src.git/+/7cdca37b049e399257d0c9e8fe657cb3724a9809

The commit message say that Safari don't have special handling but we are still failing WPT test case:

https://wpt.fyi/results/domparsing/XMLSerializer-serializeToString.html?label=master&label=experimental&aligned&view=subtest&q=serializer

We still have following in our code:

https://github.com/WebKit/WebKit/blob/285ff73b5f6d46d6e502aca30061667e41a3114b/Source/WebCore/editing/MarkupAccumulator.cpp#L512

I think aligning with WPT should be great but want to get opinion since Firefox and Safari are matching and if green signal then will go ahead then I will do PR.

Thanks!

[Radar WebKit Bug Importer]

<rdar://problem/103234827>

[Tim Nguyen (:ntim)]

We should definitely do this if it's a WPT win.

[Anne van Kesteren]

It's hard to find a test that fails, but per

https://html.spec.whatwg.org/#html-fragment-serialisation-algorithm
https://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20html%3E%0A...%3Cscript%3E%0Adocument.body.setAttributeNS(%22http%3A%2F%2Fwww.w3.org%2F1999%2Fxlink%22%2C%20%22blah%22%2C%20%22meh%22)%3B%0Aw(document.documentElement.innerHTML)%0A%3C%2Fscript%3E

this is definitely behavior that all browsers have and cannot be removed.

[Ahmad Saleem]

(In reply to Anne van Kesteren from comment #3)
> It's hard to find a test that fails, but per
> 
> https://html.spec.whatwg.org/#html-fragment-serialisation-algorithm
> https://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!
> DOCTYPE%20html%3E%0A...%3Cscript%3E%0Adocument.body.
> setAttributeNS(%22http%3A%2F%2Fwww.w3.
> org%2F1999%2Fxlink%22%2C%20%22blah%22%2C%20%22meh%22)%3B%0Aw(document.
> documentElement.innerHTML)%0A%3C%2Fscript%3E
> 
> this is definitely behavior that all browsers have and cannot be removed.

Test added by Blink patch - which is also on WPT and we are failing:

https://jsfiddle.net/sdwtqche/show

Link - http://wpt.live/domparsing/XMLSerializer-serializeToString.html

Test Name - Check if no special handling for XLink namespace unlike HTML serializer.

[Anne van Kesteren]

When I removed the line in question and compiled your test did not start passing and my test started failing. I agree there is something here, but that does not appear to be the cause. (I think Chrome's HTML handling is in a separate file.)

[Anne van Kesteren]

I looked into the failure some more:

1. Instead of generating the prefix "NS", we need to use "ns". I'm not sure when this difference was introduced, but that's what https://w3c.github.io/DOM-Parsing/#dfn-generating-a-prefix calls for.
2. Apart from that a number of failures can be attributed to attribute iteration order. WebKit and Chromium presumably use divergent hash maps and the tests rely on those in Chromium. We could address this by making the tests care less about attribute order or figure out a consistent attribute order between browsers.

Ryosuke, Chris, thoughts on 2?

[Ryosuke Niwa]

(In reply to Anne van Kesteren from comment #6)
> I looked into the failure some more:
> 
> 1. Instead of generating the prefix "NS", we need to use "ns". I'm not sure
> when this difference was introduced, but that's what
> https://w3c.github.io/DOM-Parsing/#dfn-generating-a-prefix calls for.
> 2. Apart from that a number of failures can be attributed to attribute
> iteration order. WebKit and Chromium presumably use divergent hash maps and
> the tests rely on those in Chromium. We could address this by making the
> tests care less about attribute order or figure out a consistent attribute
> order between browsers.
> 
> Ryosuke, Chris, thoughts on 2?

It seems like attribute enumeration order should be consistent across browsers. That can lead to subtle compatibility issues, right?

[Anne van Kesteren]

Indeed. It turns out element attribute order has apparently been standardized a long time ago to be in insertion order. (Thanks Ms2ger!)

However, it might be that the problem here is when we serialize xmlns:... attributes relative to other attributes.

https://w3c.github.io/DOM-Parsing/#dfn-xml-serialization-of-the-attributes seems to require xmlns:... attributes to precede the other attribute and we don't appear to be doing that.

Looks like a pretty straightforward fix I can have a look at later.

[Anne van Kesteren]

Pull request: https://github.com/WebKit/WebKit/pull/12246

[EWS]

Committed 262492@main (8e308b5b3f9e): <https://commits.webkit.org/262492@main>

Reviewed commits have been landed. Closing PR #12246 and removing active labels.