27424 2014-11-25 09:26:11 +0000 Node.isSameNode() has non-trivial usage 2016-03-16 06:21:17 +0000 1 1 1 Unclassified WebAppsWG DOM unspecified PC Linux RESOLVED FIXED P2 normal --- 1 philipj annevk bugs bzbarsky hober0 jonas mike philipj rniwa travil www-dom public-webapps-bugzilla oldest_to_newest 115444 0 philipj 2014-11-25 09:26:11 +0000 https://dom.spec.whatwg.org/#dom-node-issamenode Chrome use counter data is around 0.01%: https://www.chromestatus.com/metrics/feature/timeline/popularity/118 The implementation is trivial so there's little to gain from removal. It looks like Gecko is the only engine to not support it at all, having removed it in 2012: https://developer.mozilla.org/en-US/docs/Web/API/Node.isSameNode In IE it behaves as if: boolean isSameNode(Node other); In Blink it behaves as if: boolean isSameNode(optional Node? other = null); 115445 1 annevk 2014-11-25 09:35:08 +0000 I believe Jonas was in favor of removing this. Given that Gecko succeeded, why can't Blink? 115448 2 philipj 2014-11-25 10:15:01 +0000 It could succeed, it just seems like a harmless thing to keep around, and since it's supported everywhere except Gecko brining it back would be the simplest path to agreement between spec and browsers. Somewhat on topic, is there any data showing that isEqualNode() is required for Web compat while isSameNode() isn't? 115449 3 annevk 2014-11-25 10:18:20 +0000 Well, isSameNode() is also known as === in JavaScript. isEqualNode() is actually a novel primitive. I think that's why we decided to only try to remove the former. 115455 4 philipj 2014-11-25 10:43:33 +0000 OK, let's leave isEqualNode() out of the discussion. Jonas, how badly do you want to keep isSameNode() out of Gecko? 115469 5 bzbarsky 2014-11-25 14:02:43 +0000 I think the reasoning here was that having isSameNode was making web authors use it instead of ===, because they were afraid that the test was somehow different. Which it's not; except it's slower. 116066 6 philipj 2014-12-09 19:25:12 +0000 I gave it my best shot, but in the end we decided to neither deprecate nor remove this in Blink, on the grounds that it doesn't warrant taking even a small risk: https://groups.google.com/a/chromium.org/d/msg/blink-dev/CpIy0Pzcg4E/k0SgkniDDNUJ So, I'd like to again request that this is revived in the spec. I appreciate that Mozilla took the risk in moving first with this and many other removals, but revival really seems like the fastest way to resolve this for good. 116067 7 bzbarsky 2014-12-09 19:28:20 +0000 Quite honestly, if Blink is not willing to remove this, just like they're not willing to remove their various webkit-prefixed stuff, that doesn't mean the spec should include it, unless it's actually needed for web compat. Put another way, if we think the web compat impact of this (of which there is none that anyone knows about) requires it to be put in the spec, we should add the things that have _more_ web compat impact, like various webkit-prefixed stuff, to the spec first. 116070 8 philipj 2014-12-09 20:51:17 +0000 Yes, webkit-prefixed and other webkit-only APIs are a bigger problem, I've removed many of them and will try to remove as many more as possible. I don't know what to do about the ones required for Web compat, a Prefixes Standard seems like one way to go. I don't know if isSameNode() is required for Web compat for non-Gecko browsers, and currently have no way of finding out other than attempting the removal. If isSameNode() remains in the spec, maybe someone working on Safari or IE will have better luck. If spec'd, I think it should be isSameNode(Node? node), like isEqualNode. 116079 9 philipj 2014-12-09 23:15:51 +0000 (In reply to Philip Jägenstedt from comment #8) > If isSameNode() remains in the spec, maybe someone working on > Safari or IE will have better luck. s/in/out of/ 116090 10 annevk 2014-12-10 12:39:37 +0000 It would be interesting to know what Microsoft and Apple are doing with this method. I'm inclined to leave this bug open until either Gecko feels forced to implement this or a new browser (say Servo) feels forced to implement this. 116117 11 travil 2014-12-10 23:34:17 +0000 (In reply to Anne from comment #10) > It would be interesting to know what Microsoft and Apple are doing with this > method. I'm inclined to leave this bug open until either Gecko feels forced > to implement this or a new browser (say Servo) feels forced to implement > this. Well, IE is going to leave it in unless it's going to pose a problem (inertia is to not change). If it's ultimately not needed and Chrome can remove it (0.01 is lower than their tolerance limit of 0.03 to pull a feature out as I recall), then I think IE would feel good in removing it too. Given that, not specing it is fine by me :-) 116118 12 philipj 2014-12-10 23:47:53 +0000 I proposed removing it in Blink (and thus Chrome) but despite the 0.01% usage no amount of risk was deemed justified given how trivial this is to support, see comment #6. 116119 13 jonas 2014-12-11 00:29:23 +0000 FWIW, at mozilla we've thought of removing this function as a performance optimization. I.e. that the fallback code path that sites hopefully provide is likely faster than calling the function. I.e. we expected code like if (a.isSameNode ? a.isSameNode(b) : a === b) { // a is same as b } or if (!Node.prototype.isSameNode) { Node.prototype.isSameNode = function(other) { return this === other; }; } I don't recall if we made that assumption based on code in the wild or not. 116414 14 philipj 2014-12-17 10:35:51 +0000 Ryosuke, do you have anything to say on the prospects of removing isSameNode() in WebKit? 116420 15 rniwa 2014-12-17 14:13:08 +0000 (In reply to Philip Jägenstedt from comment #14) > Ryosuke, do you have anything to say on the prospects of removing > isSameNode() in WebKit? I don't have a strong opinion either way, and I haven't talked with my colleagues yet but my gut feeling is that we're not gonna remove this function; at least that's what I'm going to advocate if anyone suggests to remove this function in WebKit. As far as I've searched through GitHub [1], most of code samples I found that uses isSameNode are WebKit/Blink tests. This contradicts the claim that removing isSameNode is a meaningful performance optimization or that many Web authors rely on this function instead of ===. I can be convinced otherwise with a presence of more data indicating having this function is more harmful than worth the potential compatibility benefit. [1] https://github.com/search?l=javascript&p=80&q=isSameNode%28&ref=searchresults&type=Code&utf8=✓ 116421 16 rniwa 2014-12-17 14:23:31 +0000 I'll also add that one seemingly non-test code [2] I found to use isSameNode didn't seem to have any fallback. This indicates that not only is this function usage low but in a few cases where it's used, it's required for compatibility. [2] https://github.com/satisfeet/quetzal/blob/9b7dd19fa89a2084af3c12b6d32bdfdeba1fc9d4/app/products/views/show/index.js 116422 17 philipj 2014-12-17 15:34:15 +0000 Thanks, Ryosuke! Lacking any enthusiasm for removal from IE or WebKit representatives, I think isSameNode() is here to stay, however useless. Here's my spec: partial interface Node { boolean isSameNode(Node? node); } The isSameNode(node) method must return true if node and context object are the same object, and false otherwise. In Blink the argument is optional, but if the above makes it into the spec I'll fix that. 122382 18 annevk 2015-08-03 09:24:11 +0000 (In reply to Boris Zbarsky from comment #7) > Put another way, if we think the web compat impact of this (of which there > is none that anyone knows about) requires it to be put in the spec, we > should add the things that have _more_ web compat impact, like various > webkit-prefixed stuff, to the spec first. Given that we're now doing that in https://compat.spec.whatwg.org/ do you think we should add this method back? Philip, we can make the argument optional. In that case it should simply return false, right? 122397 19 philipj 2015-08-03 11:16:02 +0000 As of https://codereview.chromium.org/978223002 the argument is non-optional in Blink, matching the signature of isEqualNode(). Whether it actually needs to be nullable for web compat I don't know, but trying to find out doesn't seem worthwhile. 122407 20 bzbarsky 2015-08-03 15:17:20 +0000 > do you think we should add this method back? I don't know. I was just saying the "blink won't remove it" argument was weaker for this method than for lots of prefixed stuff is all. 125468 21 annevk 2016-03-14 13:27:02 +0000 Adding it back since Gecko can't win this alone: https://github.com/whatwg/dom/commit/97f0a958bc1dcb98265d116bafdc80ae1b059823. 😟 125470 22 philipj 2016-03-14 13:54:44 +0000 Thanks, I like the comment too. "Fortunately" there are lots more interop hazards to work through that'll require our joint efforts to resolve. 125517 23 philipj 2016-03-16 06:21:17 +0000 Tests requested in https://github.com/w3c/web-platform-tests/issues/2671