801562 - Remove Node.isSupported

Status: RESOLVED FIXED

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

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

DOM says to.  Worth a try?

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

This should land after bug 773780 to avoid bitrot.  I'll still write the patch now while I have time, and update it later.

Comment 2

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch

The diffstat made me an immediate convert:

 42 files changed, 1 insertions(+), 3484 deletions(-)

Granted that it's almost all test files -- but still, they're horribly ugly tests and I'm delighted to be rid of them.  A few more test changes might be needed.  Try: <https://tbpl.mozilla.org/?tree=Try&rev=3f35951f7d8c>

The one line inserted is because of js/xpconnect/crashtests/468552-1.html.  Bug 468552 happened to use isSupported to test for an XPConnect bug -- I switched it to replaceChild instead, on the theory that it should be testing the same basic bug.

Comment 3

[Robert Longson [:longsonr]]

For the moment perhaps the SVG mochitest should stay but use hasFeature instead and only test SVG features. That would still allow you to get rid of isSupported wouldn't it?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Given that even removing navigator.taintEnabled() (Bug 679971) caused major problems, I'd
be very careful removing this stuff.
We need to have at least some data whether it is being used.
So, please add first some telemetry probes.

Comment 5

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

(In reply to Robert Longson from comment #3)
> For the moment perhaps the SVG mochitest should stay but use hasFeature
> instead and only test SVG features. That would still allow you to get rid of
> isSupported wouldn't it?

This is written on top of bug 801425, so hasFeature() always returns true.  If bug 801425 changes so that this is no longer the case, what you suggest would make sense.

(In reply to Olli Pettay [:smaug] from comment #4)
> Given that even removing navigator.taintEnabled() (Bug 679971) caused major
> problems, I'd
> be very careful removing this stuff.
> We need to have at least some data whether it is being used.
> So, please add first some telemetry probes.

I did Google Code Search for /isSupported\([^)[]*,/ in JavaScript.  There were around 85 results and I looked at all of them -- none seemed to be real uses.  That implies at least that it's not used in any major JS library, since any major JS library would be included in at least one project indexed by Code Search.  So it's better than navigator.taintEnabled, which it turned out was used in jQuery.

We could add telemetry probes, but if so, please let's decide in advance how we plan to interpret the results.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 671362 [details] [diff] [review]
Patch

Rev iid, please.  ;)

Seems fine with that.

Comment 7

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch, rebased

I had to rewrite the patch to apply on top of the current tree, because IsSupported implementations have moved around a bunch.  It builds locally and passes some tests I tried.  I also adjusted the SVG test to use hasFeature instead of deleting it.  The basic idea is identical to the previous patch.

Try: https://tbpl.mozilla.org/?tree=Try&rev=4c0a12849b8f

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 724904 [details] [diff] [review]
Patch, rebased

r=me especially if other UAs already don't support this...

Comment 9

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Nope, we're breaking new ground here.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0e7006217d7d

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0e7006217d7d

Comment 11

[Tom S [:evilpie]]

Mentioned on https://developer.mozilla.org/en-US/docs/Firefox_22_for_developers and put an obsolete header on https://developer.mozilla.org/en-US/docs/DOM/Node.isSupported.

Comment 12

[Kohei Yoshino]

I've added this bug to the compatibility doc. Please correct the info if wrong.
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_22