Update dom/lists/DOMTokenList-iteration.html so it no longer expects duplicates #4658
Conversation
…duplicates As per the latest DOM specification [1], when a DOMTokenList gets created, we end up running the "attribute change steps" for element, localName, value, value, and null. The attribute changed steps for DOMTokenList say "set tokens to value, parsed." "parsed" links to [2] which says to run the ordered set parser, which should get rid of duplicates. WebKit, for example, does get rid of duplicates. [1] https://dom.spec.whatwg.org/#interface-domtokenlist [2] https://dom.spec.whatwg.org/#concept-ordered-set-parser
rniwa
changed the title
|
w3c-test:mirror |
|
Looks like Firefox and Chrome fail because of "expected 2 got 3" but Edge seems to fail because of the Has this recently been changed in the spec? If all engines except WebKit currently fail the test, did other vendors give one or two thumbs up? I suppose the new behavior makes sense, just checking :) |
|
Yes, I think the DOM spec has been changed. Let me try and find a reference. |
|
See whatwg/dom#105 for the relevant DOM spec discussion, such as it is. @foolip, you were in on that. The spec said something, then changed, then none of the UAs followed for years, until Safari sort of did in their pre-releses, then the spec got switched back to match shipping browsers, then Safari shipped the changed behavior (which now no longer matched the changed-back spec), then the spec got changed again to that behavior. None of the other engines have updated to that last spec change, and I'm not clear on whether they plan to, unfortunately. Speaking for Gecko, I'm still not entirely convinced on the new spec behavior, but probably willing to take a patch in Gecko if there's a patch with no performance impact, as I said in . Such a thing has not materialized yet. I myself have no plans to work on this in the near future; I have a lot of other much higher-priority things on my plate. |
|
whatwg/dom#105 is indeed related but was about the stringifier AFAICT. WebKit already reverted the change to the stringifier to match the latest spec. This particular test is not covering the stringifier. |
|
Hmm. I thought that was tracking general changes to whether the uniquifying happens, but maybe I was wrong... The general point stands: we will need a no-performance-impact change to DOMTokenList to make any changes to its behavior in Gecko, unfortunately. |
|
OK, so this is a pretty similar to the first mishap mentioned in https://blog.whatwg.org/improving-interoperability, so let's try to not do this kind of thing as much going forward. @cdumez, perhaps there are other tests for this, but can you summarize where uniquifying happens in the various engines? It looks like Blink stores the tokens separately from the underlying values, so presumably it would be fairly simple for Blink to follow WebKit here. But, if the behavior of the 3 other engines kind of makes sense, then maybe the simplest path is to change the spec again, and be very sorry about it. (Sorry if we've already discussed this elsewhere and I've forgotten.) |
|
@foolip, not completely, removes duplicates around Some time ago I rise this duplicate problem here whatwg/dom#201. Looks like Webkit finally gose like spec say (including serialising case), unfortunately still only Webkit. |
|
I thought these tests were supposed to match the spec and serve as incentive for browsers to adopt spec changes? |
|
I don't think it is a good working model to block web-platform-tests matching the spec because some implementors disagree with the specification. If those implementators want to get the specification changed, then they can try and do so. However, I don't think this should prevent web-platform-tests matching the current specification from landing until the specification gets changed. Those tests can be updated later if the spec changes (again). |
|
@cdumez I tend to agree. Do you want to address @ArkadiuszMichalski feedback here or should we do that separately? |
|
@ArkadiuszMichalski Do you mind if I do the refactoring in a follow-up? |
|
I open #4703 and explaining what is a problem, it does not lock in any way this bug (of course if removing duplicate is still on board, I assume it is because the specification has not changed in this area). |
|
If anyone takes issue with my decision here please file an issue at https://github.com/whatwg/dom/issues/new and I will make sure it gets the attention it deserves, but I didn't feel blocking @cdumez longer was warranted. |
|
FWIW, I tried running all the tests in http://w3c-test.org/dom/lists/ in Chrome, Edge, Firefox and Safari. Safari Technology Preview passed them all, but elsewhere there are widespread failures. I had to do some manual checking on Edge, but in the end it seems like Chrome, Edge and Firefox do no uniquifying. Since Safari is now passing everything, I'd feel like a bit of a douche trying to change it all back, so I guess let's see if anyone else picks up on the change. |
As per the latest DOM specification [1], when a DOMTokenList gets created, we
end up running the "attribute change steps" for element, localName, value, value,
and null. The attribute changed steps for DOMTokenList say "set tokens to value,
parsed." "parsed" links to [2] which says to run the ordered set parser, which
should get rid of duplicates. WebKit, for example, does get rid of duplicates.
[1] https://dom.spec.whatwg.org/#interface-domtokenlist
[2] https://dom.spec.whatwg.org/#concept-ordered-set-parser