Remove Document#selectedStylesheetSet/preferredStylesheetSet |
|||||||||||
Issue descriptionCurrently in Blink Document.idl, we have: attribute DOMString? selectedStylesheetSet; readonly attribute DOMString? preferredStylesheetSet; But in the spec, it has: [SameObject] readonly attribute StyleSheetList styleSheets; Should we do something to match the spec, either update the spec or change our implementation?
,
Feb 13 2017
https://drafts.csswg.org/cssom/#extensions-to-the-document-interface This link is right above the "stylesheet" attributes in Blink Document.idl (https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Document.idl?rcl=28ff32abc83ae8939f56702151b3d982859d56b5&l=145). If the link is no longer correct, please update the link. Thanks
,
Mar 9 2017
,
Apr 4 2017
,
Apr 4 2017
,
Apr 4 2017
,
Oct 30 2017
CL to add use counters for the currently shipped attributes: https://chromium-review.googlesource.com/c/chromium/src/+/745322
,
Oct 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/708cfbd0e177072433ccf56307f0982101502af5 commit 708cfbd0e177072433ccf56307f0982101502af5 Author: Eric Willigers <ericwilligers@chromium.org> Date: Tue Oct 31 08:02:23 2017 Document: Use counters for stylesheetSet attributes We add use counters for the following attributes: attribute DOMString? selectedStylesheetSet; readonly attribute DOMString? preferredStylesheetSet; BUG= 690600 , 690609 Change-Id: I0edf7fc6ec5e3e9295d36f18ad5853ffabfa6c1e Reviewed-on: https://chromium-review.googlesource.com/745322 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Bugs Nash <bugsnash@chromium.org> Commit-Queue: Eric Willigers <ericwilligers@chromium.org> Cr-Commit-Position: refs/heads/master@{#512775} [add] https://crrev.com/708cfbd0e177072433ccf56307f0982101502af5/third_party/WebKit/LayoutTests/fast/dom/document-preferredStylesheetSet-use-counter.html [add] https://crrev.com/708cfbd0e177072433ccf56307f0982101502af5/third_party/WebKit/LayoutTests/fast/dom/document-selectedStylesheetSet-use-counter-001.html [add] https://crrev.com/708cfbd0e177072433ccf56307f0982101502af5/third_party/WebKit/LayoutTests/fast/dom/document-selectedStylesheetSet-use-counter-002.html [modify] https://crrev.com/708cfbd0e177072433ccf56307f0982101502af5/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/708cfbd0e177072433ccf56307f0982101502af5/third_party/WebKit/Source/core/dom/Document.idl [modify] https://crrev.com/708cfbd0e177072433ccf56307f0982101502af5/third_party/WebKit/public/platform/web_feature.mojom [modify] https://crrev.com/708cfbd0e177072433ccf56307f0982101502af5/tools/metrics/histograms/enums.xml
,
Nov 27 2017
Marking as BlockedBug as we need to wait for use counter data
,
Nov 27 2017
I'm guessing we'll get enough data around April 2018.
,
Dec 6 2017
,
Apr 1 2018
The NextAction date has arrived: 2018-04-01
,
Apr 22 2018
For the getters, it looks like we're at about 0.1% to 0.17% of page loads. For the setter of selectedStyleSheet, it bounces between 0% and probably one or two page loads.
,
May 4 2018
cnardi@, what's your takeaway from those numbers? Will both the getter and setter be affected by matching the spec, and is there a way to estimate how much of that 0.17% of the getter might result in real breakage?
,
May 4 2018
So the setter seems like something we could drop for sure, since (virtually) nothing uses it. However, the getters are a more complicated situation it seems. The parts of the spec that our code claims to implement were removed in https://github.com/w3c/csswg-drafts/commit/e5353d078e686136fdb28c7de37ffd375cd6d79f, citing https://www.w3.org/Bugs/Public/show_bug.cgi?id=29471 and https://lists.w3.org/Archives/Public/www-style/2013Aug/0640.html. This notes that the parts of that removed section which were implemented by Blink/WebKit are non-standard in that they are preferredStylesheetSet and selectedStylesheetSet vs preferredStyleSheetSet and selectedStyleSheetSet (note the difference in the casing of stylesheet). It also seems like nothing has changed since that change, as Firefox still implements all of those methods per https://dxr.mozilla.org/mozilla-central/source/dom/webidl/Document.webidl#296. So to sum it up: we and WebKit still implement the non-standard preferredStylesheetSet and selectedStylesheetSet methods (but I'm not sure if the behavior of these non-standard methods aligns with the old spec). Firefox implements the standard methods which were removed in https://github.com/w3c/csswg-drafts/commit/e5353d078e686136fdb28c7de37ffd375cd6d79f. And Edge doesn't appear to implement any of these methods.
,
May 4 2018
Firefox bug on removing these methods: https://bugzilla.mozilla.org/show_bug.cgi?id=1260720 (old) WebKit bug about the difference in the casing of "Stylesheet": https://bugs.webkit.org/show_bug.cgi?id=62407
,
May 4 2018
Also note that some of my comments above would apply to bug 690600 too, since I was making comments about both methods.
,
May 5 2018
If you're looking into dropping all these, I could look into removing them from Firefox, I wasn't aware of that bug, but it allows me to clean up some related code :)
,
May 5 2018
I think it makes sense to remove the two methods we implement, since usage seems to be fairly low. However, it would probably be good to verify that the breakage from removing them would be fairly minimal.
,
May 5 2018
Sent https://github.com/w3c/web-platform-tests/pull/10861 to create some failing tests. selectedStylesheetSet also needs a use counter, and both need to be moved to the non-standard section of our document.idl.
,
May 5 2018
We actually do have use counters for all of these attributes, the ones for selectedStylesheetSet are just separated out into getter/setter and thus not implemented in IDL. https://www.chromestatus.com/metrics/feature/timeline/popularity/2211 - DocumentGetPreferredStylesheetSet https://www.chromestatus.com/metrics/feature/timeline/popularity/2212 - DocumentGetSelectedStylesheetSet https://www.chromestatus.com/metrics/feature/timeline/popularity/2213 - DocumentSetSelectedStylesheetSet
,
May 5 2018
Issue 690600 has been merged into this issue.
,
May 5 2018
Ah, didn't realize that :) I think that some httparchive analysis would be a reasonable next step, to figure out what the usage looks like. Some tips for that are here: https://www.chromium.org/blink/platform-predictability/compat-tools
,
May 6 2018
Philip, do you think you could help me with getting access to run httparchive queries on my Chromium account? Since I'm not a Googler, it doesn't let me, and I already ran one query this month on my personal account.
,
May 7 2018
I don't think we have a way to hook up Chromium accounts with BigQuery billing, which is very unfortunate. To move this along, I checked if the counters in #21 were hit on any site, and they were not. Used this query: https://docs.google.com/document/d/1cpjWFoXBiuFYI4zb9I7wHs7uYZ0ntbOgLwH-mgqXdEM/edit#heading=h.3701s9nnsb27 I can get you a list of URLs with matches for selectedStylesheetSet and preferredStylesheetSet, would that help?
,
May 7 2018
Yeah, that would probably help. Although if the counters weren't hit, there probably won't be that many matches for the attributes themselves. Thanks!
,
May 7 2018
OK, there were actually incredibly few hits. selectedStylesheetSet (https://bigquery.cloud.google.com/savedquery/762219082167:235fd43fbe7647ea94ec7b14498e173b): http://www.instructorscorner.org/ http://www.deltadentalcoversme.com/ http://www.wellsfargocommunity.com/ preferredStylesheetSet (https://bigquery.cloud.google.com/savedquery/762219082167:26eff7fb22a147408e113694ec2dd17b): http://www.pokolenie-x.com/ http://www.instructorscorner.org/ http://www.wellsfargocommunity.com/ http://www.deltadentalcoversme.com/ Looking into these might tell us something interesting about the risks of removal. But this really is exceptionally few results, so I think the 0.17% must be either to accidentally hitting the counter, or a single large-ish site. Both are good news that mean lower risk than what 0.17% "normally" means.
,
May 7 2018
Delta Dental and Wells Fargo are two major US companies, so it would make sense that the use counts were inflated. However, it doesn't seem like these sites actually use either selectedStylesheetSet or preferredStylesheetSet. The only hit I could find in either site was at https://www.deltadentalcoversme.com/s/sfsites/auraFW/javascript/1bO4dJePbDnoI-_VdhdsEQ/aura_prod.js and https://www.wellsfargocommunity.com/s/sfsites/auraFW/javascript/1bO4dJePbDnoI-_VdhdsEQ/aura_prod.js, but they're only referenced in a dictionary mapping some values to attributes on different prototypes. https://www.instructorscorner.org/s/sfsites/auraFW/javascript/1bO4dJePbDnoI-_VdhdsEQ/aura_proddebug.js appears to be an un-minified version of the same file. The last site (http://www.pokolenie-x.com/) uses https://getfirebug.com/firebug-lite.js, which is only for debugging, and appears to be unmaintained. Because of this, I don't believe there should be any compatibility risk from removing both attributes. I may have missed some use of selectedStylesheetSet or preferredStylesheetSet in the aura files, but from my viewing it looks like they are only referenced and not used.
,
May 7 2018
Thanks for checking it out. If you'd like to send an intent to deprecate and remove, I'll like to say LGTM :)
,
May 9 2018
Philip, do you want to just go ahead with immediate removal per https://groups.google.com/a/chromium.org/d/msg/blink-dev/w1Bv7YZxAco/91dOdEkiCAAJ, or go with the deprecation route you suggested a couple messages before?
,
May 9 2018
I would prefer removal without deprecation in this case. If we're right, almost all of the deprecation warnings will be bogus.
,
May 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/62e388ac4d084b73649d99df72e651559be44077 commit 62e388ac4d084b73649d99df72e651559be44077 Author: Chris Nardi <cnardi@chromium.org> Date: Thu May 10 17:37:08 2018 Remove Document#selectedStylesheetSet/preferredStylesheetSet Document#selectedStylesheetSet/preferredStylesheetSet are non-standard methods that are only implemented by WebKit and Blink. Their standard versions are no longer in the spec. Remove them entirely from our implementation. Intent to Deprecate and Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/w1Bv7YZxAco Bug: 690609 Change-Id: I796a6bc0693934eb308a4fe59b52e4a9d1269e6f Reviewed-on: https://chromium-review.googlesource.com/1049051 Commit-Queue: Chris Nardi <cnardi@chromium.org> Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#557569} [delete] https://crrev.com/1b5126a68c8620c18c515d5aa1e7bc61394c99af/third_party/WebKit/LayoutTests/external/wpt/css/cssom/historical-expected.txt [add] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/WebKit/LayoutTests/external/wpt/css/cssom/preferred-stylesheet-order.html [add] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/WebKit/LayoutTests/external/wpt/css/cssom/preferred-stylesheet-reversed-order.html [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/WebKit/LayoutTests/external/wpt/shadow-dom/untriaged/shadow-trees/upper-boundary-encapsulation/test-011.html [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/WebKit/LayoutTests/fast/css/link-disabled-attr-expected.txt [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/WebKit/LayoutTests/fast/css/link-disabled-attr.html [delete] https://crrev.com/1b5126a68c8620c18c515d5aa1e7bc61394c99af/third_party/WebKit/LayoutTests/fast/css/preferred-stylesheet-order-expected.txt [delete] https://crrev.com/1b5126a68c8620c18c515d5aa1e7bc61394c99af/third_party/WebKit/LayoutTests/fast/css/preferred-stylesheet-order.html [delete] https://crrev.com/1b5126a68c8620c18c515d5aa1e7bc61394c99af/third_party/WebKit/LayoutTests/fast/css/preferred-stylesheet-reversed-order-expected.txt [delete] https://crrev.com/1b5126a68c8620c18c515d5aa1e7bc61394c99af/third_party/WebKit/LayoutTests/fast/css/preferred-stylesheet-reversed-order.html [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/WebKit/LayoutTests/fast/dom/document-attribute-js-null-expected.txt [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/WebKit/LayoutTests/fast/dom/document-attribute-js-null.html [delete] https://crrev.com/1b5126a68c8620c18c515d5aa1e7bc61394c99af/third_party/WebKit/LayoutTests/fast/dom/document-preferredStylesheetSet-use-counter.html [delete] https://crrev.com/1b5126a68c8620c18c515d5aa1e7bc61394c99af/third_party/WebKit/LayoutTests/fast/dom/document-selectedStylesheetSet-use-counter-001.html [delete] https://crrev.com/1b5126a68c8620c18c515d5aa1e7bc61394c99af/third_party/WebKit/LayoutTests/fast/dom/document-selectedStylesheetSet-use-counter-002.html [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/blink/public/platform/web_feature.mojom [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/blink/renderer/core/css/style_engine.cc [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/blink/renderer/core/css/style_engine.h [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/blink/renderer/core/dom/document.cc [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/blink/renderer/core/dom/document.h [modify] https://crrev.com/62e388ac4d084b73649d99df72e651559be44077/third_party/blink/renderer/core/dom/document.idl
,
May 10 2018
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by mikelawther@chromium.org
, Feb 12 2017