Intent to deprecate and Remove: currentView, useCurrentView and SVGViewSpec

127 views
Skip to first unread message

Shanmuga Pandi M

unread,
Jul 21, 2016, 2:02:36 AM7/21/16
to blink-dev

Intent to deprecate and Remove: currentView, useCurrentView and SVGViewSpec

 

Primary eng (and PM) emails

shanm...@samsung.com


Summary

Deprecate and Remove the SVGViewSpec interface and currentView, useCurrentView of SVGSVGElement Interface.

 

Motivation

It has been removed from the SVG 2.0 specification:

 

https://github.com/w3c/svgwg/commit/4c26fd36937a65192024208d85c144a21071b057
https://www.w3.org/Graphics/SVG/WG/track/actions/3800?changelog


Compatibility Risk

UseCounter  is indicating very low usage. (only zeros reported)

 

Usage information from UseCounter

https://www.chromestatus.com/metrics/feature/timeline/popularity/1036

 

OWP launch tracking bug

No OWP launch tracking bug but there is:

https://bugs.chromium.org/p/chromium/issues/detail?id=629445

Entry on the feature dashboard

https://www.chromestatus.com/features/4511711998509056

Requesting approval to remove too?

Yes


Chris Harrelson

unread,
Jul 21, 2016, 7:24:11 PM7/21/16
to Shanmuga Pandi M, blink-dev
LGTM1

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

TAMURA, Kent

unread,
Jul 21, 2016, 10:23:43 PM7/21/16
to Chris Harrelson, Shanmuga Pandi M, blink-dev
--
TAMURA Kent
Software Engineer, Google


Philip Jägenstedt

unread,
Jul 21, 2016, 11:27:46 PM7/21/16
to TAMURA, Kent, Chris Harrelson, Shanmuga Pandi M, blink-dev
The SVGSVGElementFragmentSVGViewElement counter in SVGSVGElement::inheritViewAttributes is also relevant, measuring how often the view comes from an SVGViewElement. I suspect that removing currentView, useCurrentView and all of SVGViewSpec (not just the IDL file) would result in SVGViewElement becoming entirely unused or at least useless. But it's still in the SVG spec.

Shanmuga, do you have a work-in-progress CL for how things will look after the removals? If SVGViewElement really becomes unused, then we should consider removing it at the same time and having it removed from the spec as well.

Shanmuga

unread,
Jul 22, 2016, 12:21:32 AM7/22/16
to blink-dev, tk...@chromium.org, chri...@chromium.org, shanmu...@gmail.com

@Philip,

This is the WIP patch.
https://codereview.chromium.org/2163213007/

But as per the SVG spec "#svgView()" still supported.
So we just need to remove only SVGViewSpec interface , I think.

Fredrik Söderquist

unread,
Jul 22, 2016, 4:04:51 AM7/22/16
to Philip Jägenstedt, TAMURA, Kent, Chris Harrelson, Shanmuga Pandi M, blink-dev
On Fri, Jul 22, 2016 at 5:27 AM, Philip Jägenstedt <foo...@chromium.org> wrote:
The SVGSVGElementFragmentSVGViewElement counter in SVGSVGElement::inheritViewAttributes is also relevant, measuring how often the view comes from an SVGViewElement. I suspect that removing currentView, useCurrentView and all of SVGViewSpec (not just the IDL file) would result in SVGViewElement becoming entirely unused or at least useless. But it's still in the SVG spec.

That's not quite how these things tie together - SVGViewSpec pretty much just reflects settings made by other means. Getting rid of will mean less restrictions on the underlying code though, so there are definitely cleanups to be made. I'd support removal of SVGViewElement, but I don't see it falling out this removal by means of being dead code. (Which naturally doesn't prevent expanding the intent. I'll also mention the related https://www.chromestatus.com/metrics/feature/timeline/popularity/1036 - which I find to be surprisingly low...)


/fs

dk...@google.com

unread,
Jul 26, 2016, 2:19:16 PM7/26/16
to blink-dev, foo...@chromium.org, tk...@chromium.org, chri...@chromium.org, shanmu...@gmail.com
Is this blocked on API owner feedback?

Philip Jägenstedt

unread,
Jul 26, 2016, 10:18:22 PM7/26/16
to dk...@google.com, blink-dev, tk...@chromium.org, chri...@chromium.org, shanmu...@gmail.com
Yes, I left it hanging, sorry.

I'm trying to understand what group of things makes sense to remove together and the motivation for removing this from the SVG spec. I end up at https://www.w3.org/Graphics/SVG/WG/track/actions/3800 but am unable to find more context.

What is suspect to me in https://codereview.chromium.org/2163213007/ is that only surface-level changes were made, the only internal simplifications are some helpers in SVGViewSpec.cpp that went away. This doesn't seems like a reasonable end state, so I'm wondering if there's a next step where the real wins are?

I was interested to find that Gecko has only useCurrentView but no currentView attribute, while Edge doesn't have either, and neither has the SVGViewSpec interface. But does this correspond to some functionality being missing as well?

Fredrik Söderquist

unread,
Jul 27, 2016, 2:32:02 PM7/27/16
to Philip Jägenstedt, dk...@google.com, blink-dev, Kent Tamura, Chris Harrelson, shanmuga pandi
On Wed, Jul 27, 2016 at 4:18 AM, Philip Jägenstedt <foo...@chromium.org> wrote:
Yes, I left it hanging, sorry.

I'm trying to understand what group of things makes sense to remove together and the motivation for removing this from the SVG spec. I end up at https://www.w3.org/Graphics/SVG/WG/track/actions/3800 but am unable to find more context.



What is suspect to me in https://codereview.chromium.org/2163213007/ is that only surface-level changes were made, the only internal simplifications are some helpers in SVGViewSpec.cpp that went away. This doesn't seems like a reasonable end state, so I'm wondering if there's a next step where the real wins are?

I believe I outlined some possible "future work" in the CL mentioned above. Nothing earth-shattering, but generally a simpler setup.
 
I was interested to find that Gecko has only useCurrentView but no currentView attribute, while Edge doesn't have either, and neither has the SVGViewSpec interface. But does this correspond to some functionality being missing as well?

(The test mentioned in the minutes linked above might give some indication of this.)


/fs

Philip Jägenstedt

unread,
Jul 28, 2016, 11:06:19 PM7/28/16
to Fredrik Söderquist, dk...@google.com, blink-dev, Kent Tamura, Chris Harrelson, shanmuga pandi
Even with the list of suggested simplification I was not sure about how much code it would actually amount to, so I tried applying Shanmuga's CL and then also ripping out SVGViewElement. AFAICT, it's really only SVGViewElement.* and a chunk of SVGSVGElement::setupInitialView that goes away. Notably, nothing about the internal useCurrentView/currentView changes. It seems to me that unless support for fragment identifiers and the viewBox+zoomAndPan attributes are dropped, which isn't on the table, then internally the simplification will be modest in the end.

Does this sound about right? If so, then I think it's just a question of how best to reach interop here. The API isn't entirely useless AFAICT, one can at least tell what the viewport is, or is that information available by other means? I still lean towards removal, but perhaps with a very long deprecation period.

Fredrik Söderquist

unread,
Jul 29, 2016, 1:07:00 PM7/29/16
to Philip Jägenstedt, Dru Knox, blink-dev, Kent Tamura, Chris Harrelson, shanmuga pandi
On Fri, Jul 29, 2016 at 5:06 AM, Philip Jägenstedt <foo...@chromium.org> wrote:
Even with the list of suggested simplification I was not sure about how much code it would actually amount to, so I tried applying Shanmuga's CL and then also ripping out SVGViewElement. AFAICT, it's really only SVGViewElement.* and a chunk of SVGSVGElement::setupInitialView that goes away. Notably, nothing about the internal useCurrentView/currentView changes. It seems to me that unless support for fragment identifiers and the viewBox+zoomAndPan attributes are dropped, which isn't on the table, then internally the simplification will be modest in the end.

There'd likely be more simplifications one could make, but that would involve some internal restructuring, so won't be entirely with effort (i.e it'll not be just ripping out code.)
 
Does this sound about right? If so, then I think it's just a question of how best to reach interop here. The API isn't entirely useless AFAICT, one can at least tell what the viewport is, or is that information available by other means? I still lean towards removal, but perhaps with a very long deprecation period.

The information exposed via SVGViewSpec is not directly available through other means, no. (Some is available through SVGSVGElement in a sense, but because of current bugs w.r.t SVGViewSpec it might be a stretch to say that.)


/fs

Philip Jägenstedt

unread,
Jul 29, 2016, 4:25:43 PM7/29/16
to Fredrik Söderquist, Dru Knox, blink-dev, Kent Tamura, Chris Harrelson, shanmuga pandi
OK, so given the state of currentView, useCurrentView and SVGViewSpec in Gecko and Edge and the usage, this removal is very likely safe compat-wise, and a win for interop if they're also removed in WebKit eventually. Towards that end, a svg/historical.html in web-platform-tests and a WebKit issue would be very useful.

Since it's possible that someone is using this information for something useful, I suggest deprecation now and removal in M56. Shanmuga, does that sounds reasonable to you? LGTM1 assuming no new information comes to light.

Rick Byers

unread,
Jul 29, 2016, 4:52:24 PM7/29/16
to Philip Jägenstedt, Fredrik Söderquist, Dru Knox, blink-dev, Kent Tamura, Chris Harrelson, shanmuga pandi
Thanks for the discussion everyone.

LGTM4 (Philip is apparently living in GF(2)+1 today).

Shanmuga

unread,
Aug 1, 2016, 1:49:39 AM8/1/16
to blink-dev, f...@opera.com, dk...@google.com, tk...@chromium.org, chri...@chromium.org, shanmu...@gmail.com


On Saturday, July 30, 2016 at 1:55:43 AM UTC+5:30, Philip Jägenstedt wrote:
OK, so given the state of currentView, useCurrentView and SVGViewSpec in Gecko and Edge and the usage, this removal is very likely safe compat-wise, and a win for interop if they're also removed in WebKit eventually. Towards that end, a svg/historical.html in web-platform-tests and a WebKit issue would be very useful.

Since it's possible that someone is using this information for something useful, I suggest deprecation now and removal in M56. Shanmuga, does that sounds reasonable to you? LGTM1 assuming no new information comes to light.


    It is reasonable to me. I will make deprecation now and removal in M56.
Thank you all.
 
Reply all
Reply to author
Forward
0 new messages