Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always update a DOMTokenList #444

Merged
merged 1 commit into from Apr 26, 2017
Merged

Always update a DOMTokenList #444

merged 1 commit into from Apr 26, 2017

Conversation

annevk
Copy link
Member

@annevk annevk commented Apr 19, 2017

In particular when invoking toggle() and replace(). Also rewrite it in
terms of the Infra Standard.

Fixes #442 and fixes #443.


Preview | Diff

In particular when invoking toggle() and replace(). Also rewrite it in
terms of the Infra Standard.

Fixes #442 and fixes #443.
@annevk
Copy link
Member Author

annevk commented Apr 19, 2017

@ayg I think this fixes both your issues, assuming I write a suitable fix for the Infra Standard, which I'll work on.

@ayg
Copy link
Contributor

ayg commented Apr 19, 2017

Thanks!

@annevk
Copy link
Member Author

annevk commented Apr 19, 2017

That is now whatwg/infra#126. Review appreciated here and there.

@annevk
Copy link
Member Author

annevk commented Apr 19, 2017

Oh, and since you were writing tests, I should maybe include a pointer to those in the commit message.

@ayg
Copy link
Contributor

ayg commented Apr 19, 2017

I have a patch submitted in Gecko-land. I have comments marking where to change when the two issues I filed are resolved. If you want a link to the file, it's here (raw).

@ayg
Copy link
Contributor

ayg commented Apr 19, 2017

I briefly reviewed both of the PRs, and at a glance they LGTM.

@ayg
Copy link
Contributor

ayg commented Apr 19, 2017

To clarify, the test I'm submitting does not match the new spec and will have to be updated, which I'm happy to do. There's no point in updating the in-tree test because I'm rewriting it anyway and you're just going to make jgraham sad when he has to figure out how to merge it.

@annevk
Copy link
Member Author

annevk commented Apr 26, 2017

@ayg will you update the tests?

@annevk annevk deleted the annevk/domtokenlist branch April 26, 2017 08:04
@ayg
Copy link
Contributor

ayg commented Apr 26, 2017

Tests submitted upstream. Note that I'm disappearing as of tonight, so it's possible they won't actually get accepted for a few months. If this happens and someone wants them before then, feel free to slurp them into wpt directly. (I'll try to remember to submit a wpt PR or at least ask someone to if this happens, but I'm posting it here too so people know in case I forget.)

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 26, 2017
…hods r=masayuki

Previously, replace() and toggle() would not always remove duplicates
and whitespace from the DOM attribute in the case where they were no-ops
(replacing a nonexistent token, force-toggling a token to its current
state).  Now they do.  This matches the behavior of add() and remove(),
and also replace() in one case (replacing an existing token with
itself).

This follows a spec change: whatwg/dom#444

MozReview-Commit-ID: 7lDEQxOGxPV

--HG--
extra : rebase_source : 842ff24c46681649affcedcba2623128fc6f5a7b
@smaug----
Copy link
Collaborator

Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1360230 some changes here aren't web compatible.

jryans pushed a commit to jryans/gecko-dev that referenced this pull request May 3, 2017
…hods r=masayuki

Previously, replace() and toggle() would not always remove duplicates
and whitespace from the DOM attribute in the case where they were no-ops
(replacing a nonexistent token, force-toggling a token to its current
state).  Now they do.  This matches the behavior of add() and remove(),
and also replace() in one case (replacing an existing token with
itself).

This follows a spec change: whatwg/dom#444

MozReview-Commit-ID: 7lDEQxOGxPV
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
…hods r=masayuki

Previously, replace() and toggle() would not always remove duplicates
and whitespace from the DOM attribute in the case where they were no-ops
(replacing a nonexistent token, force-toggling a token to its current
state).  Now they do.  This matches the behavior of add() and remove(),
and also replace() in one case (replacing an existing token with
itself).

This follows a spec change: whatwg/dom#444

MozReview-Commit-ID: 7lDEQxOGxPV

UltraBlame original commit: 47b53e3d4b27c0b900c06180d857dac1de1d3241
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
…hods r=masayuki

Previously, replace() and toggle() would not always remove duplicates
and whitespace from the DOM attribute in the case where they were no-ops
(replacing a nonexistent token, force-toggling a token to its current
state).  Now they do.  This matches the behavior of add() and remove(),
and also replace() in one case (replacing an existing token with
itself).

This follows a spec change: whatwg/dom#444

MozReview-Commit-ID: 7lDEQxOGxPV

UltraBlame original commit: 47b53e3d4b27c0b900c06180d857dac1de1d3241
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
…hods r=masayuki

Previously, replace() and toggle() would not always remove duplicates
and whitespace from the DOM attribute in the case where they were no-ops
(replacing a nonexistent token, force-toggling a token to its current
state).  Now they do.  This matches the behavior of add() and remove(),
and also replace() in one case (replacing an existing token with
itself).

This follows a spec change: whatwg/dom#444

MozReview-Commit-ID: 7lDEQxOGxPV

UltraBlame original commit: 47b53e3d4b27c0b900c06180d857dac1de1d3241
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants