Add isSameNode() back to Node

RESOLVED FIXED in Firefox 48

Status

()

defect
RESOLVED FIXED
3 years ago
a month ago

People

(Reporter: smaug, Assigned: jdai, Mentored)

Tracking

({dev-doc-complete})

36 Branch
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [tw-dom])

Attachments

(2 attachments, 6 obsolete attachments)

(Reporter)

Updated

3 years ago
Whiteboard: [tw-dom]
Mentor: bzbarsky
(Assignee)

Comment 1

3 years ago
I would like to try on this bug.
Assignee: nobody → jdai
(Assignee)

Comment 2

3 years ago
Attachment #8733246 - Flags: review?(bzbarsky)
(Assignee)

Comment 3

3 years ago
Attachment #8733247 - Flags: review?(bzbarsky)
Comment on attachment 8733246 [details] [diff] [review]
Bug 1256299 - Implement Node.isSameNode.

>+nsINode::IsSameNode(nsIDOMNode* aOther, bool* aReturn)

You don't need this function.

>+  NS_IMETHOD IsSameNode(nsIDOMNode* aArg, bool* aResult) __VA_ARGS__ override \

Or this bit.

>+++ b/dom/interfaces/core/nsIDOMNode.idl

Because you don't want to add anything to this file.  nsIDOMNode is deprecated; we should slowly be removing things from it, not adding them.

r=me with that fixed.  Thank you for picking this up!
Attachment #8733246 - Flags: review?(bzbarsky) → review+
Comment on attachment 8733247 [details] [diff] [review]
Bug 1256299 - Remove redundent trailing spaces.

r=me
Attachment #8733247 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 6

3 years ago
Thanks for your review and valuable feedback. I addressed all of them into my patch.
Attachment #8733246 - Attachment is obsolete: true
Attachment #8733823 - Flags: review+
(Assignee)

Comment 7

3 years ago
Attachment #8733247 - Attachment is obsolete: true
Attachment #8733824 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
adding 1256299 to series file
renamed 1256299 -> bug_1256299_1.patch
applying bug_1256299_1.patch
patching file dom/base/nsINode.cpp
Hunk #1 FAILED at 970
1 out of 1 hunks FAILED -- saving rejects to file dom/base/nsINode.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh bug_1256299_1.patch

could you take a look, thanks!
Flags: needinfo?(jdai)
Keywords: checkin-needed
(Assignee)

Comment 10

3 years ago
Sorry for the inconvenience, I didn't expect my patches can't apply to m-i. I will fix that and re-land my patch.
Flags: needinfo?(jdai)
(Assignee)

Comment 11

3 years ago
I didn't modify any code, only changed patch order. Those patches can apply on m-c and m-i.

Try result:https://treeherder.mozilla.org/#/jobs?repo=try&revision=32b78ffb4284&filter-tier=1
Attachment #8733824 - Attachment is obsolete: true
Attachment #8734680 - Flags: review+
(Assignee)

Comment 12

3 years ago
Attachment #8733823 - Attachment is obsolete: true
Attachment #8734681 - Flags: review+
(Assignee)

Comment 13

3 years ago
Attachment #8734681 - Attachment is obsolete: true
Attachment #8734685 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 16

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7c6bed47abd4
https://hg.mozilla.org/mozilla-central/rev/2706aaa4d078
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Reopening the docs part. 

Eric: there is no example in https://developer.mozilla.org/en-US/docs/Web/API/Node/isSameNode and the spec table is the 2010 version (and no browser compat table) in https://developer.mozilla.org/en-US/docs/Web/API/Node/isEqualNode

Also in all the Node pages sidebar, isSameNode is still marked as deprecated.

Also add the bug number to the entry in Firefox 48 for developers.

Thank you.
Flags: needinfo?(eshepherd)
Yeah, true! All I did was update the compatibility information -- I made no other changes to the page. I intentionally avoided making other changes because when I do, you tend to complain that I'm deviating from my deliverables by doing more than necessary to close the specific bug at hand. :)

I'll start doing more thorough updates to these pages going forward when I touch them.
Flags: needinfo?(eshepherd)
OK, all of this should now be fixed.
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.