Closed Bug 1444909 Opened 7 years ago Closed 7 years ago

Return boolean from DOMTokenList's replace() method

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: code, Assigned: bzbarsky)

References

Details

(Keywords: dev-doc-complete)

Attachments

(3 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Firefox/60.0 Build ID: 20180311220116 Steps to reproduce: A proposed change to the specification for DOMTokenList's replace() method which would make it return a boolean - indicating true if a token was replaced and false otherwise - has received positive responses from implementers. Pull request and discussion: https://github.com/whatwg/dom/pull/582 Tests: https://github.com/w3c/web-platform-tests/pull/9920
Thank you for filing this and for writing the tests!
Assignee: nobody → bzbarsky
Status: UNCONFIRMED → NEW
Ever confirmed: true
See Also: → 1443998
This fixes upcoming tests that will check when we actually mutate the attribute. MozReview-Commit-ID: 7r5ErK9wvir
Attachment #8958706 - Flags: review?(kyle)
The wpt changes come from https://github.com/w3c/web-platform-tests/pull/9920 and are needed to keep Element-classlist.html passing. The change to testing/web-platform/mozilla/tests/dom/classList.html is pulling those changes into our weird forked version of that test... MozReview-Commit-ID: CvQlBRuieUY
Attachment #8958707 - Flags: review?(kyle)
Attachment #8958706 - Flags: review?(kyle) → review+
Attachment #8958707 - Flags: review?(kyle) → review+
Attachment #8958708 - Flags: review?(kyle) → review+
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7702f7024085 part 1. Fix DOMTokenList.replace() to not do the attr set when the old token was not found. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/c97167cf4845 part 2. Change DOMTokenList.replace() to return a boolean and pull in the corresponding web platform test updates. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/bb405d71ba67 part 3. Add testing for what mutation observers happen when doing DOMTokenList.replace. r=qdot
Blocks: 1443998
James, any idea why the test changes here did not get upstreamed to wpt?
Flags: needinfo?(james)
It seems like 5a1c088ef04548f623a2b6704466158eaf300c9f upstream already contains these changes, which was a situation that's badly handled (we error out rather than do something more reasonable like nothing).
Flags: needinfo?(james)
Where "these changes" is "some but not all of the changes from this bug", right?
Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/10183 for changes under testing/web-platform/tests
I have documented this: Description of replace return value updated: https://developer.mozilla.org/en-US/docs/Web/API/DOMTokenList/replace Note added to Fx61 rel notes: https://developer.mozilla.org/en-US/Firefox/Releases/61#DOM PR for updating the browser compat data: https://github.com/mdn/browser-compat-data/pull/2124 Let me know if that look OK, thanks!
Flags: needinfo?(bzbarsky)
Chris, the returned value is a boolean value, not a Boolean object. The documentation all seems to claim a Boolean object is returned, which isn't right. Other than that, looks good.
Flags: needinfo?(bzbarsky)
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #13) > Chris, the returned value is a boolean value, not a Boolean object. The > documentation all seems to claim a Boolean object is returned, which isn't > right. > > Other than that, looks good. Ooops. I've fixed it now, thanks for reporting.
Looks great, thank you!
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: