Closed Bug 1174097 Opened 9 years ago Closed 2 years ago

remove SVGSVGElement.useCurrentView

Categories

(Core :: SVG, task)

task

Tracking

()

RESOLVED FIXED
107 Branch
Tracking Status
firefox41 --- wontfix
firefox107 --- fixed

People

(Reporter: heycam, Assigned: longsonr)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) (deleted) — Splinter Review
SVGSVGElement.currentView and useCurrentView have been dropped from SVG 2. The former we never implemented. https://github.com/w3c/svgwg/commit/4c26fd36937a65192024208d85c144a21071b057 r?longsonr for the SVG change, r?smaug for the IDL change.
Attachment #8621481 - Flags: review?(longsonr)
Attachment #8621481 - Flags: review?(bugs)
Assignee: nobody → cam
Status: NEW → ASSIGNED
Comment on attachment 8621481 [details] [diff] [review] patch There's no point in having a valid field if you're removing the code that checks it. Nor is there much point in syntax checking fragments since we can't tell if they are valid without this interface. I'm not sure how much the test_fragments.html test amounts to without that. I can't think of any other way to test fragments though.
Attachment #8621481 - Flags: review?(longsonr) → review-
I'm guess (reluctantly since I don't see what harm there is in having the method) I'm OK with removing test_fragments altogether if this is the way we're going, since there won't really be any way to test whether they do anything except to see if there's a visual change. What do you think Daniel?
Flags: needinfo?(dholbert)
mUseCurrentView seems to be used internally still (in SVGFragmentIdentifier.cpp), so I think we should keep it.
My previous comments were only about the test_fragments.html testcase. I agree that mUseCurrentView needs to stay.
Comment on attachment 8621481 [details] [diff] [review] patch Do we have any evidence we can remove the property? Other browsers seem to have this and supporting useCurrentView doesn't seem to cost us much. So, is it ok to remove it?
I have never personally seen any pages that use this property (and it's not a particularly useful feature IMO), but I haven't measured anything. You are right that it doesn't cost much but if we can remove a feature nobody uses then I'd prefer to. Other vendors in the F2F meeting this week agreed. IE doesn't support this. Once bug 968923 is done, we could measure this, if you think we should.
Comment on attachment 8621481 [details] [diff] [review] patch I think we should wait for some usage data before removing this property.
Attachment #8621481 - Flags: review?(bugs)
(In reply to Robert Longson from comment #3) > I'm guess (reluctantly since I don't see what harm there is in having the > method) I'm OK with removing test_fragments altogether if this is the way > we're going, since there won't really be any way to test whether they do > anything except to see if there's a visual change. > > What do you think Daniel? I think this makes sense (modulo comment 9). Though if we end up removing this test, it sounds like we should replace it with some reftests, to test for the visual change that we're expecting. (or make sure we already have such reftests)
Flags: needinfo?(dholbert)
Blocks: svg2
This was removed in Chrome 56 Intent to deprecate and Remove: currentView, useCurrentView and SVGViewSpec https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Dfqld7wjHJM
Type: defect → task

Not working on this, but I am certain this is safe to remove.

Assignee: cam → nobody
Status: ASSIGNED → NEW
Severity: normal → S3
Assignee: nobody → longsonr
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 107 Branch

The (minimal) FF107 docs work for this can be tracked in https://github.com/mdn/content/issues/21539

Attachment #8621481 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: