Closed Bug 1295404 Opened 3 years ago Closed 3 months ago

remove requiredFeatures from SVGTests

Categories

(Core :: SVG, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: longsonr, Assigned: longsonr)

References

(Blocks 1 open bug)

Details

(Keywords: site-compat)

Attachments

(1 file, 4 obsolete files)

They've been removed from SVG 2
Blocks: 1216893
Assignee: nobody → longsonr
Comment on attachment 8781350 [details] [diff] [review]
WIP - still needs various tests to be fixed or removed

Should I remove hasExtension() from SVGTests here or in another bug?
Attachment #8781350 - Flags: feedback?(cam)
Comment on attachment 8781350 [details] [diff] [review]
WIP - still needs various tests to be fixed or removed

Review of attachment 8781350 [details] [diff] [review]:
-----------------------------------------------------------------

I found bug 1295404 for removing hasExtension(), but I guess I overlooked sending that mail.  In that bug you mentioned we should do this removal behind a pref, presumably because we would be the first ones removing this.  (I just checked, and Chrome/Safari/Edge all still expose SVGTests.requiredFeatures.)

From foolip's comment in https://bugs.chromium.org/p/chromium/issues/detail?id=635420 it sounds like the Blink use counter is sufficiently low for SVGTests in general.  And my gut feeling is that it's safe.  But if you think we should still put this behind a pref for now, then I'm still in favour of making hasFeature() just return true all the time (which would solve bug 1216893's issue).  We should probably still send an intent to unimplement, either way.
Attachment #8781350 - Flags: feedback?(cam) → feedback+
bug 1295404 is this bug.

I think I'll morph this into HasFeature to return true. Then the other bug (whatever it is) can remove it.
Oops, bug 1133175 is for hasExtension.
Priority: -- → P3
Attached patch updated patch (obsolete) — Splinter Review
Attachment #8781350 - Attachment is obsolete: true
Blocks: 1529564

Chrome has already removed all interfaces in svg/historical.html: https://wpt.fyi/results/svg/historical.html

We probably should also remove the remaining 3 interfaces.

Well I'm not sure...
useCurrentView is actually pretty useful to us for our own tests. I'd rather leave it as it's hard to test fragment identifiers otherwise.
getTransformToElement is pretty harmless, there's a polyfill somewhere that adds it back.
w3c are still talking about SVGZoomAndPan - https://lists.w3.org/Archives/Public/www-svg/2019Jun/0014.html maybe that will end up being removed altogether since that one really is useless as we don't send any zoom/pan events.

So I'd rather fix other things really.

Attached patch 1295404.txt (obsolete) — Splinter Review
Attachment #9073325 - Attachment is obsolete: true
Attached patch 1295404.txt (obsolete) — Splinter Review
Attachment #9073388 - Attachment is obsolete: true
Attachment #9073395 - Attachment is obsolete: true

https://www.w3.org/TR/SVG2/struct.html#ConditionalProcessingOverview

Previous versions of SVG included a third conditional processing attribute, requiredFeatures. This was intended to allow authors to provide fallback behavior for user agents that only implemented parts of the SVG specification. Unfortunately, poor specification and implementation of this attribute made it unreliable as a test of feature support.

Chrome removed it some time ago.

bzbarsky for the webidl change

Chrome removed it some time ago.

Does Safari support it?

Flags: needinfo?(longsonr)
Flags: needinfo?(longsonr)
Pushed by longsonr@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba32151f8fc1
Remove requiredFeatures from SVGTests r=dholbert r=bzbarsky
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.