New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 674593



Sign in to add a comment
link

Issue 635420: Deprecate and Remove SVGTests.requiredFeatures attribute

Reported by shanmug...@samsung.com, Aug 8 2016 Project Member

Issue description

In SVG2, hasFeature always return true.
So requiredFeatures attribute not doing anything useful.

It has been removed from the spec:
https://github.com/w3c/svgwg/commit/9a30d01f6410dc516c5f874d71e957230a3448cd
https://www.w3.org/2015/06/09-svg-irc#T15-35-40
 

Comment 1 by shanmug...@samsung.com, Aug 8 2016

Status: Assigned (was: Untriaged)

Comment 2 by foolip@chromium.org, Aug 8 2016

Summary: Deprecate and Remove SVGTests.requiredFeatures attribute (was: Depercate and Remove SVGTests.requiredFeatures attribute)

Comment 3 by shanmug...@samsung.com, Aug 8 2016

This attribute is not having seperate UseCounter, measuring the usage with useCounter is the first step as asked on blink-dev?

Comment 4 by schenney@chromium.org, Aug 8 2016

I'd like to remove the un-used DOMImplementationHasFeature and DOMImplementationHasFeatureReturnsFalse UseCounter enums, and you might be able to use the enum values for the new enums you'll add for requiredFeature. You should check the procedure for removing UseCounter enums before going ahead with my suggestion.

Comment 5 by foolip@chromium.org, Aug 8 2016

Those enum values are already gone from UseCounter.h, but in any case you should normally not reuse entries, just add a new one.

But in this case, this is covered by a use counter with low usage, so I wouldn't bother trying to split it further:
https://www.chromestatus.com/metrics/feature/timeline/popularity/1072

You should probably also investigate if requiredExtensions and systemLanguage really make sense, or if they could be removed too, which would require a spec change.

Comment 6 by shanmug...@samsung.com, Aug 9 2016

I will further split the usecounter.
https://www.chromestatus.com/metrics/feature/timeline/popularity/1072 

and for the two `return false` branches of SVGTests::isValid.

Comment 7 by schenney@chromium.org, Jan 3 2017

Any action on this, shanmuga@?

Comment 8 by shanmug...@samsung.com, Jan 4 2017

Status: Available (was: Assigned)
Ah.. Sorry.. I have occupied with some other work. Keep it available now.

Comment 9 by shanmug...@samsung.com, Jan 4 2017

Owner: ----

Comment 10 by foolip@chromium.org, Jan 4 2017

There's a test for its removal in https://github.com/w3c/web-platform-tests/blob/master/svg/historical.html which is also imported to Blink.

Comment 11 by f...@opera.com, Feb 24 2017

Cc: lunalu@chromium.org

Comment 12 by foolip@chromium.org, Feb 27 2017

Blocking: 674593

Comment 13 by foolip@chromium.org, Mar 9 2017

Conclusion on https://github.com/w3c/svgwg/issues/308 was to keep it out of the spec. Whether or not that's the right call, this will now need some action on our part to move forward.

Comment 14 by rijubrat...@intel.com, Mar 9 2017

I have a WIP CL here (https://codereview.chromium.org/2741463002/), if we decide to remove this.

Comment 15 by foolip@chromium.org, Mar 20 2017

#14, if you want to assign this to yourself and drive the deprecation/removal, that'd be great. The next step would be to send an Intent to Deprecate and Remove to blink-dev. requiredFeatures doesn't do anything, and usage is bounded by https://www.chromestatus.com/metrics/feature/timeline/popularity/1072

One milestone of deprecation seems like a good idea, but no need for two separate intents for the deprecation and removal.

Comment 16 by rijubrat...@intel.com, Mar 24 2017

Owner: rijubrat...@intel.com
Status: Started (was: Available)

Comment 17 by bugdroid1@chromium.org, Mar 28 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6fbe0502934c1d8d4f72c4197e10d2b99879c85

commit e6fbe0502934c1d8d4f72c4197e10d2b99879c85
Author: rijubrata.bhaumik <rijubrata.bhaumik@intel.com>
Date: Tue Mar 28 16:26:31 2017

SVGStringList update tests.

Update the SVGStringList tests to use a SVGStringList other than requiredFeatures,
as we need to remove requiredFeatures.
Fix SVGStringList-basics.
Make SVGStringList.js use requiredExtensions instead of requiredFeatures.

BUG= 635420 

Review-Url: https://codereview.chromium.org/2780443002
Cr-Commit-Position: refs/heads/master@{#460120}

[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/external/wpt/svg/interfaces-expected.txt
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/external/wpt/svg/interfaces.html
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/linux/svg/W3C-SVG-1.1-SE/svgdom-over-01-f-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/linux/svg/W3C-SVG-1.1-SE/svgdom-over-01-f-expected.txt
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/linux/svg/dom/SVGStringList-basics-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/mac-mac10.10/svg/dom/SVGStringList-basics-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/mac-mac10.11/svg/W3C-SVG-1.1-SE/svgdom-over-01-f-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/mac-mac10.9/svg/dom/SVGStringList-basics-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/mac-retina/svg/W3C-SVG-1.1-SE/svgdom-over-01-f-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-SVG-1.1-SE/svgdom-over-01-f-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-SVG-1.1-SE/svgdom-over-01-f-expected.txt
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/mac/svg/dom/SVGStringList-basics-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/win/svg/W3C-SVG-1.1-SE/svgdom-over-01-f-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/win/svg/W3C-SVG-1.1-SE/svgdom-over-01-f-expected.txt
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/platform/win/svg/dom/SVGStringList-basics-expected.png
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/svg/W3C-SVG-1.1-SE/svgdom-over-01-f.svg
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/svg/dom/SVGStringList-basics-expected.txt
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/svg/dom/SVGStringList-basics.xhtml
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/svg/dom/SVGStringList-expected.txt
[modify] https://crrev.com/e6fbe0502934c1d8d4f72c4197e10d2b99879c85/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGStringList.js

Comment 18 by bugdroid1@chromium.org, Mar 31 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3bc8c04a9e2e711079299a591fa9eadc56b09729

commit 3bc8c04a9e2e711079299a591fa9eadc56b09729
Author: rijubrata.bhaumik <rijubrata.bhaumik@intel.com>
Date: Fri Mar 31 07:14:25 2017

Remove SVGTests.requiredFeatures attribute

In SVG2, hasFeature always return true.
So requiredFeatures attribute not doing anything useful.

It has been removed from the spec:
https://github.com/w3c/svgwg/commit/9a30d01f6410dc516c5f874d71e957230a3448cd
https://www.w3.org/2015/06/09-svg-irc#T15-35-40

Intent to Deprecate : https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/qiFyionxCYg

BUG= 635420 

Review-Url: https://codereview.chromium.org/2782553006
Cr-Commit-Position: refs/heads/master@{#461065}

[modify] https://crrev.com/3bc8c04a9e2e711079299a591fa9eadc56b09729/android_webview/tools/system_webview_shell/test/data/webexposed/global-interface-listing-expected.txt
[delete] https://crrev.com/b4cab81eb31fc479ab7583d6cbfab9288750aeb5/third_party/WebKit/LayoutTests/external/wpt/svg/historical-expected.txt
[modify] https://crrev.com/3bc8c04a9e2e711079299a591fa9eadc56b09729/third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/3bc8c04a9e2e711079299a591fa9eadc56b09729/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
[delete] https://crrev.com/b4cab81eb31fc479ab7583d6cbfab9288750aeb5/third_party/WebKit/LayoutTests/svg/W3C-SVG-1.1-SE/types-dom-06-f.svg
[modify] https://crrev.com/3bc8c04a9e2e711079299a591fa9eadc56b09729/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/3bc8c04a9e2e711079299a591fa9eadc56b09729/third_party/WebKit/Source/core/svg/SVGTests.cpp
[modify] https://crrev.com/3bc8c04a9e2e711079299a591fa9eadc56b09729/third_party/WebKit/Source/core/svg/SVGTests.h
[modify] https://crrev.com/3bc8c04a9e2e711079299a591fa9eadc56b09729/third_party/WebKit/Source/core/svg/SVGTests.idl

Comment 19 by f...@opera.com, Apr 4 2017

Fixed?

Comment 20 by rijubrat...@intel.com, Apr 4 2017

Comment#4: asked to remove DOMImplementationHasFeature also.

WIP CL: https://codereview.chromium.org/2787853002/
A lot of tests were failing, and need to be removed.

If we don't include C#4 in this bucket of work, then its fixed and i can land the WIP CL separately sometime later.

Comment 21 by f...@opera.com, Apr 4 2017

I don't think we should be doing that, see https://codereview.chromium.org/1108293002 for additional context. (Also, note that the use-counter was removed by that CL...)

Comment 22 by rijubrat...@intel.com, Apr 4 2017

Ah! thanks for the context. We can mark this as fixed then.

Comment 23 by rijubrat...@intel.com, Apr 4 2017

Status: Fixed (was: Started)

Sign in to add a comment