Closed Bug 687400 Opened 8 years ago Closed 8 years ago
This function simply implements the '==' operator. For us it's just useless code we're carrying around, and for JS authors it's directly harmful as it's slower than simply using '=='. Patch attached which removes it.
Attachment #560844 - Flags: review?(Olli.Pettay)
Should we first warn about removal? Google code search shows that isSameNode is used occasionally. (Although at least in some cases the code is absolutely bizarre; something like if (node.isSameNode ? node.isSameNode(otherNode) : node == otherNode) ... )
I was thinking about that too. It'd be easy to warn if we're fine with removing the custom quickstubs that we currently have, and then put the removal in next release. Let me know if you prefer that?
Try run for cda7df7c1a15 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=cda7df7c1a15 Results (out of 27 total builds): success: 25 warnings: 2 Builds available at http://email@example.com
(In reply to Jonas Sicking (:sicking) from comment #2) > I was thinking about that too. It'd be easy to warn if we're fine with > removing the custom quickstubs that we currently have, and then put the > removal in next release. What has quickstubs to do with the warning?
Comment on attachment 560844 [details] [diff] [review] Patch to fix But we really must warn about the removal and have the warning at least during one release.
Attachment #560844 - Flags: review?(Olli.Pettay) → review+
Olli, feel free to land this if it passes review.
Assignee: nobody → jonas
Attachment #562689 - Flags: review?(Olli.Pettay)
Attachment #562689 - Flags: review?(Olli.Pettay) → review+
Is this patch a request to remove the method outright or to rewire what it does internally? If it's an outright removal request what is === compatibility in older browsers contrasting isSameNode? I am finding this invaluable in helping me iron out a Gecko range/selection bug.
You can always use === to test if two nodes are the same node. So |a.isSameNode(b)| will produce the same result as |a === b| in all versions of gecko. Well.. except that some older versions of gecko doesn't support the former. So the latter is strictly an improvement. And it executes faster too.
Jonas, my question is if this bug is a request to remove the method outright or to rewire what it does internally? If you've requested an outright removal of the method then can you please point out which version of the W3C DOM specification where it has been deprecated?
It's a request to outright remove it as an experiment to see if it can be done without "breaking the web". The spec editor is waiting for feedback from that experiment before removing it. Please search the public-webapps archives for "isSameNode".
Then you have determined that the === operator is EXPLICITLY intended to work with comparing nodes? If so please reference where in which specification and version it is explicitly stated.
j.j., please clarify which point and how it's wording applies DIRECTLY to context of matching nodes in the manner isSameNode works. Jonas, Firefox does not exist for you to personally experiment with to see if you can break the web. If you are concerned about performance on other people's websites then your bug report should have been limited to suggesting a warning be made in the console in strict mode at best.
[shrug] 7. Return true if x and y refer to the same object. Otherwise, return false. Not enough?
Incorrect reference, that is under 11.9.3 (==), NOT 11.9.4 (===).
Sorry, 11.9.6 step 7
John, if you need an EXPLICIT note in the current W3C DOM specification, please let me know and I'll make it happen.
> note in the current W3C DOM Isn't that "language-neutral"?
Checked in to inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/82c201d75331
8 years ago
https://hg.mozilla.org/mozilla-central/rev/82c201d75331 taking a brief look at this bug's history looks like all patches landed.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [leave open for remaining patch]
Target Milestone: --- → mozilla10
The removal of this method is now breaking scripts on my site. Thanks for not supporting standards and breaking the web!
Actually, this was done to better support cross browser behaviour since not all browsers support this function. It was also done to help JS authors as == is faster than isSameNode. Also,isSameNode is likely going to to be removed from the specs, so there's only advantages to stop using it.
DOM4 has been updated accordingly.
Most add-on developers (and reviewers) test their add-ons looking for error messages, not warnings. Deprecation messages that we know we'll follow through should be errors IMO, to make them easier to spot. I see several add-ons on AMO that still use isSameNode, so I think we should extend the deprecation period with an error instead.
Though I personally usually don't use isSameNode() method (but I _did_ use on server side), I believe that its removal is a wrong step. DOM levels should be incremental, and if something is (to be) removed from new DOM4, this does not mean that it should be removed from existing implementations. Stable version of a major browser is noway a place for such experiments. You are free to mark it as obsolete feature, but you should not remove it.
I just got the message "TypeError: elt.isSameNode is not a function" in Firefox 8. Did someone back-port this "fix"? (Also, I suspect there may be a bug in comparing nodes with "==" when working through proxy objects in add-ons. But I haven't confirmed this yet.)
Disregard previous message re FF 8. My bad.
(In reply to Marat Tanalin | tanalin.com from comment #27) > Though I personally usually don't use isSameNode() method (but I _did_ use > on server side), I believe that its removal is a wrong step. > > DOM levels should be incremental, and if something is (to be) removed from > new DOM4, this does not mean that it should be removed from existing > implementations. Stable version of a major browser is noway a place for such > experiments. > > You are free to mark it as obsolete feature, but you should not remove it. As one of the editors of DOM4, I can say that it is the intention that features that were removed from the specification are also removed from implementations, in order to simplify the platform as a whole. If features are to remain in implementations, they will remain in the specification as well.
It's back to the DOM spec: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27424 https://github.com/whatwg/dom/commit/97f0a958bc1dcb98265d116bafdc80ae1b059823 so now will back to Gecko?
Please file new bugs for new issues ;)
You need to log in before you can comment on or make changes to this bug.