Closed Bug 1254956 Opened 8 years ago Closed 8 years ago

Implement Node.rootNode

Categories

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

36 Branch
defect
Not set
normal

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
Whiteboard: [tw-dom]
Assignee: nobody → joliu
FYI, there are some web-platform-tests coming from Apple.
Trying to find a link to those
(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...
(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.
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.
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+
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)
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 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+
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
https://hg.mozilla.org/mozilla-central/rev/3bc53f14f51e
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Doc updates underway. Will finish this evening.
Depends on: 1269155
See Also: → 1270387
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.