| Summary: | Serialize xmlns attributes first | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
| Component: | DOM | Assignee: | Anne van Kesteren <annevk> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | annevk, cdumez, rniwa, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | Safari Technology Preview | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
|
Description
Ahmad Saleem
2022-12-04 18:06:46 PST
We should definitely do this if it's a WPT win. 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. (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. 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.) 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? (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? 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. Pull request: https://github.com/WebKit/WebKit/pull/12246 Committed 262492@main (8e308b5b3f9e): <https://commits.webkit.org/262492@main> Reviewed commits have been landed. Closing PR #12246 and removing active labels. |