Closed
Bug 1254956
Opened 8 years ago
Closed 8 years ago
Implement Node.rootNode
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: smaug, Assigned: yrliou)
References
Details
(Keywords: dev-doc-complete, Whiteboard: [tw-dom])
Attachments
(1 file)
This is a recent addition to the DOM spec. Webkit and Blink are implementing it. Internally we have nsINode::SubtreeRoot() which, if I recall correctly, should map exactly to Node.rootNode
Reporter | ||
Updated•8 years ago
|
Whiteboard: [tw-dom]
Updated•8 years ago
|
Updated•8 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → joliu
Reporter | ||
Comment 1•8 years ago
|
||
FYI, there are some web-platform-tests coming from Apple. Trying to find a link to those
Reporter | ||
Comment 2•8 years ago
|
||
https://github.com/w3c/web-platform-tests/pull/2665
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #2) > https://github.com/w3c/web-platform-tests/pull/2665 I added |RootNode()| method which basically just call |SubtreeRoot()|. And applied this patch under testing/web-platform/tests, no new fail cases when I ran ./mach web-platform-tests testing/web-platform/tests/dom/interfaces.html. But when I ran ./mach web-platform-tests testing/web-platform/tests/dom/nodes/rootNode.html, it said "No testharness tests to run" and 0 tests were ran. Not sure what I'm missing here...
Assignee | ||
Comment 4•8 years ago
|
||
(In reply to Jocelyn Liu [:jocelyn] [:joliu] from comment #3) > (In reply to Olli Pettay [:smaug] from comment #2) > > https://github.com/w3c/web-platform-tests/pull/2665 > > I added |RootNode()| method which basically just call |SubtreeRoot()|. > And applied this patch under testing/web-platform/tests, no new fail cases > when I ran ./mach web-platform-tests > testing/web-platform/tests/dom/interfaces.html. > > But when I ran ./mach web-platform-tests > testing/web-platform/tests/dom/nodes/rootNode.html, it said "No testharness > tests to run" and 0 tests were ran. > Not sure what I'm missing here... OK, it turns out to be a silly one, I forgot to update the manifest. Now all tests are passed.
Reporter | ||
Comment 5•8 years ago
|
||
I think we could then land the patch (after a review) and just wait a tiny bit to get those automated tests for free. wpt->mozilla-central merge should happen quite often.
Assignee | ||
Comment 6•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/40771/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40771/
Attachment #8731677 -
Flags: review?(bugs)
Reporter | ||
Comment 7•8 years ago
|
||
Comment on attachment 8731677 [details] MozReview Request: Bug 1254956 - Implement Node.rootNode. r?smaug, r=Ms2ger https://reviewboard.mozilla.org/r/40771/#review37277
Attachment #8731677 -
Flags: review?(bugs) → review+
Comment 10•8 years ago
|
||
backed out for problems like https://treeherder.mozilla.org/logviewer.html#?job_id=24216639&repo=mozilla-inbound
Flags: needinfo?(joliu)
Assignee | ||
Comment 11•8 years ago
|
||
Sorry for the inconvenience, I didn't notice that web-platform test is already synced to our repo and marked as expected FAIL, will fix that and re-land my patch.
Flags: needinfo?(joliu)
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8731677 [details] MozReview Request: Bug 1254956 - Implement Node.rootNode. r?smaug, r=Ms2ger Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40771/diff/1-2/
Comment 13•8 years ago
|
||
Comment on attachment 8731677 [details] MozReview Request: Bug 1254956 - Implement Node.rootNode. r?smaug, r=Ms2ger https://reviewboard.mozilla.org/r/40771/#review37849 Stealing... r=smaug on the code changes, r=me on the test expectation updates.
Attachment #8731677 -
Flags: review+
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8731677 [details] MozReview Request: Bug 1254956 - Implement Node.rootNode. r?smaug, r=Ms2ger Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40771/diff/2-3/
Attachment #8731677 -
Attachment description: MozReview Request: Bug 1254956 - Implement Node.rootNode. r?smaug → MozReview Request: Bug 1254956 - Implement Node.rootNode. r?smaug, r=Ms2ger
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bc53f14f51e
Keywords: checkin-needed
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3bc53f14f51e
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 17•8 years ago
|
||
Doc updates underway. Will finish this evening.
Comment 18•8 years ago
|
||
Updated: https://developer.mozilla.org/en-US/docs/Web/API/Node/rootNode https://developer.mozilla.org/en-US/docs/Web/API/Node#Properties Updated Firefox 48 for developers.
Keywords: dev-doc-needed → dev-doc-complete
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•