This is a painfully useless feature. Could we try always returning true? With any luck, that will put us always in the "good" code path. It's possible that it will break enough sites that we have to make exceptions, but it seems like a way better strategy than trying to spec all the feature strings that compat requires. Perhaps more importantly, it should serve as a clear indicator to authors that the function is useless and shouldn't be used! Ms2ger is apparently fine with this, so if Anne is too, we'd love to change the spec. We just need UAs willing to take a small risk and see if any sites break.

Do we have any data how often we return false currently?

Should I make nsSVGFeatures::HasFeature always return true too? It seems to be used by a test file. Why does nsDOMAttribute::IsSupported warn but nothing else does? I don't see a warning in the console anyway when I test, although maybe I'm doing it wrong. Try, since I'm guessing this will cause at least one mochitest failure: https://tbpl.mozilla.org/?tree=Try&rev=5eb11043eb4e

Comment on attachment 671232 [details] [diff] [review] Patch Review of attachment 671232 [details] [diff] [review]: ----------------------------------------------------------------- Worth a try, I think.

ccing Jonas and David for their thoughts.

I'm fine with doing this if the spec is changed to say that this is the new specced behavior. I agree that these functions are useless, but it's a little iffy to go our own way against the spec anyway.

(In reply to Jonas Sicking (:sicking) from comment #5) > I'm fine with doing this if the spec is changed to say that this is the new > specced behavior. I agree that these functions are useless, but it's a > little iffy to go our own way against the spec anyway. Anne, Ms2ger, and I are all happy to change the spec if someone (e.g., Gecko) is willing to try it out. I'll do that (and land official tests) before I land any change to Gecko, so we'll match the spec. On a related note: the DOM spec says that isSupported shouldn't exist. Anne thinks this is better in the long term because it's one less useless function, and that it's tenable because practically no pages use isSupported (unlike hasFeature). Are we interested in doing this? My feeling is that maintaining a one-line function forever is negligible cost, and there's no point in breaking even a tiny number of pages to gain such a tiny simplification.

(In reply to :Aryeh Gregor from comment #6) > On a related note: the DOM spec says that isSupported shouldn't exist. Anne > thinks this is better in the long term because it's one less useless > function, and that it's tenable because practically no pages use isSupported > (unlike hasFeature). Are we interested in doing this? My feeling is that > maintaining a one-line function forever is negligible cost, and there's no > point in breaking even a tiny number of pages to gain such a tiny > simplification. We've already broken a few pages by removing functions such as xmlVersion. If we will never remove it, it should be added to the spec.

Let's worry about whether to remove it or not in a separate bug, and for now just make the functions return true.

I just removed the failing dom-level2-core tests outright. They're horrible and we should get rid of as many as possible. I also got rid of Ms2ger's test because it will be changed or replaced upstream when the spec change lands, which will be before this patch lands, so there's no point in trying to register all the new failures. We'll get the new upstream tests when we next sync. No changes here other than to tests, relative to the last patch version.

(This also bitrots bug 773780. :( I think that has to land soon if we don't want to block all Node-related work on it!)

There are some w3c svg testsuite tests for hasFeature e.g. http://www.w3.org/Graphics/SVG/Test/20110816/harness/htmlObject/struct-dom-02-b.html http://www.w3.org/Graphics/SVG/Test/20110816/harness/htmlObject/struct-dom-03-b.html http://www.w3.org/Graphics/SVG/Test/20110816/harness/htmlObject/struct-dom-04-b.html Perhaps you should suggest to the svg w3c group that these be removed/deprecated? Also feature strings are used in svg by http://www.w3.org/TR/SVG/struct.html#ConditionalProcessing and http://www.w3.org/TR/SVG/feature.html If hasFeature is going away then would need updating for SVG2 wouldn't it? In the meantime won't it be odd that hasFeature returns true but when you create an SVG element with a requiredFeatures attribute it might or might not display? I'm not suggesting that feature strings are good/useful and a legitimate solution may be to rip them them out of SVG and/or say that requiredFeatures never disables any SVG element whether gecko has implemented that part of the specification or not.

I'm not familiar with SVG's use of hasFeature. From what you're saying, and glancing at the spec, it looks like SVG uses feature strings for declarative feature-testing, and uses hasFeature() to match. This might well be worth keeping around. If so, the DOM spec needs to special-case SVG. Perhaps it could say that any feature strings beginning with "http://www.w3.org/TR/SVG" or "org.w3c.svg" returns true or false depending on what the SVG spec says, and all others return true. Does that make sense to everyone?

We return true/false for SVG feature strings dependent on what things we've implemented. For instance we haven't implemented SVG fonts so we return false for http://www.w3.org/TR/SVG11/feature#Font Comment 12 makes sense to me.

> I think that has to land soon if we don't want to block all Node-related work on it! Well, I think it could have landed today if someone hadn't bitrotted it in spite of an explicit request to the contrary. But yes, it needs to land soon, period. And it's way higher priority than any other Node change I've seen coming through, honestly.

The code being removed looks pretty sketchy -- were these two APIs really supposed to have the same implementation? In any case, I don't think this API is useful given that nothing makes us keep the API results in sync with the code.

(In reply to David Baron [:dbaron] from comment #15) > The code being removed looks pretty sketchy -- were these two APIs really > supposed to have the same implementation? I think so, yes. The old standard seems fairly vague about whether there's supposed to be any difference: http://www.w3.org/TR/DOM-Level-3-Core/core.html#DOMFeatures I guess the theory might be that some features could be supported for one node but not another, but I don't see anything from a glance at the spec that suggests anything specific, so making them always the same appears legit. > In any case, I don't think this API is useful given that nothing makes us > keep the API results in sync with the code. Comment 11 says that SVG uses this feature mechanism declaratively, so that you can define fallback in a general way for browsers that don't support a certain feature. It's similar to the various feature-testing methods often proposed for CSS, except that in CSS it's usually not hard to design properties to have good fallback given CSS error handling rules, but I guess that's not the case in SVG. Given that the declarative SVG feature looks useful, it makes sense to have a programmatic way to access the same info, for SVG. hasFeature() already works for this purpose, and is used in the wild, so it makes sense to me to keep it for this. On the other hand, hasFeature() is not useful at all for DOM features, because it's almost always more intuitive and reliable and fine-grained to just check whether the relevant properties exist, or call them and catch any exceptions, etc. So at this point I'd suggest returning true for all non-SVG features, and keeping the existing functionality for SVG, as outlined in comment 12. Does that sound good to everyone?

I guess that sounds ok to me. > I think that has to land soon if we don't want to block all Node-related work on it! Peter is aiming to do that tomorrow.

Comment on attachment 671357 [details] [diff] [review] Patch with updated tests, for less orange! r- pending updated patch.

Anne has agreed to the SVG exception. I'll update the spec and add some upstream tests now. Try: https://tbpl.mozilla.org/?tree=Try&rev=ca3d86940dca

Comment on attachment 671811 [details] [diff] [review] Patch v3, doesn't disable SVG feature-testing r=me

Try is green, spec and test changes are here: https://github.com/whatwg/dom/commit/cdf75764431b41dd2e3e6ca671b39ca3c8d55262 http://dvcs.w3.org/hg/webapps/rev/a9725c7b0fd1 And there's no conflict with bug 773780. https://hg.mozilla.org/integration/mozilla-inbound/rev/f73f203effde

Documented: https://developer.mozilla.org/en-US/docs/Firefox_19_for_developers#DOM https://developer.mozilla.org/en-US/docs/DOM/Node.isSupported#Gecko-specific_notes https://developer.mozilla.org/en-US/docs/DOM/document.implementation#Gecko-specific_notes