Skip to content

[cssom] Figure out what to do with non-standard CSSStyleSheet methods in WebKit / Blink #3814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
emilio opened this issue Apr 8, 2019 · 5 comments · Fixed by #3900
Closed
Labels
cssom-1 Current Work

Comments

@emilio
Copy link
Collaborator

emilio commented Apr 8, 2019

WebKit / Blink implement some legacy Microsoft CSSOM functions on CSSStyleSheet, which if I'm not wrong come from Microsoft:

https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/css/css_style_sheet.idl?l=37-40&rcl=6dc816e543555790648ec5af1ca827431a7d5807

  • rules just calls cssRules.
  • removeRule just calls deleteRule.
  • addRule just string-concats a selector and a style block and calls insertRule.

It's not too hard to add them to Gecko, but addRule in particular seems rather hacky / inconsistent with the rest of the APIs. We haven't seen any compat issue due to not having them (but I just got bit by it when trying to test something).

Would there be any chance on collecting data and potentially removing them? Or is all hope lost / is not worth the churn and should I just put them in the spec?

@emilio emilio added cssom-1 Current Work Agenda+ labels Apr 8, 2019
@ewilligers
Copy link
Contributor

They already have Blink use counters:

rules - 0.97%
addRule - 0.36%
removeRule - 0.34%

Usage is likely too high for removal.

@tabatkins
Copy link
Member

Yeah, those numbers are too high, absent some convincing evidence that it's all some detection library and not true usage. I am curious about Firefox not getting compat bugs tho.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Figure out what to do with non-standard CSSStyleSheet methods in WebKit / Blink.

The full IRC log of that discussion <dael> Topic: Figure out what to do with non-standard CSSStyleSheet methods in WebKit / Blink
<dael> github: https://github.com//issues/3814
<dael> Rossen_: Anyone to take this?
<emilio> Given the comments, I'll just spec them
<dael> TabAtkins: Based on eric's numbers, unless someone can make argument that our numbers aren't actual usage they're too high to drop. .rules is almost a full % of page loads
<dael> TabAtkins: If not removing, should look to add. But emilio says no one cares on their side. I don't know best thing, but removing isn't in the cards for us
<dael> AmeliaBR: Clarify impl of these they're aliases for standard features?
<dael> TabAtkins: There's one that's slightly different, but yes
<dael> dbaron: Will impl these in browsers that don't have them break?
<dbaron> s/break/break feature detection/
<dael> TabAtkins: Don't know. I jsut have use counter data, not information on how they're actually used. But numbers os use is too high
<dael> dbaron: Seems weird b/c I don't recall pages broken due to not having them
<dael> TabAtkins: Feature detection I didn't mean tell you're in blink but something that iterates over a bunch of properties to see what exists. That's caused high use counters in the past. THat might be the case at which point we might remove, but as of right now can't remove
<dael> Rossen_: Other action if we don't remove? Add to CSSOM?
<dael> TabAtkins: Not right now. emilio asked if should or should remove
<dael> Rossen_: Current data says cannot remove unless you want an action to see if you can get deeper into data to figure out if it's meaningful or triggering from aux. detection rules. Up to you
<dael> TabAtkins: Put it for triage. I'll get back to you
<dael> Rossen_: Reasonable.

@emilio
Copy link
Collaborator Author

emilio commented May 8, 2019

Just got a webcompat issue due to this, I'll just implement these in Gecko and spec them. https://bugzilla.mozilla.org/show_bug.cgi?id=1545823

@dbaron
Copy link
Member

dbaron commented May 8, 2019

The CSS WG just discussed this:

RESOLVED: Spec these methods, mark them deprecated, add ADVISEMENT to authors to not teach or use

09:48:32 <fantasai> Topic: non-standard CSSStyleSheet methods
09:48:32 <@dbaron> Topic: webkit/blink methods on CSSStyleSheet
09:48:48 <florian> emilio: I got a compat issue report on this
09:48:57 <florian> emilio: I'm planning to implement and spec
09:49:09 <florian> emilio: it is sad that we need them, but it is also easy, so I'll just do it
09:49:37 <florian> emilio: anyone has a strong concern about adding this
09:49:39 <florian> ?
09:50:04 <florian> plinss: I have been advocating for pushing such things to a side spec / compat spec
09:50:21 mode: @fantasai (opped) and @florian (opped)  
09:50:28 <@florian> TabAtkins: no, I want the compat spec to go away, it should be in the main spec
09:50:40 <@florian> plinss: that encourage new content to use it
09:51:02 <@florian> fantasai: we can mark them as deprecated, presented as a footnote, make warnings, etc
09:51:11 <@florian> fantasai: but still put them in the main spec
09:51:21 <chris> I agree that deprecation is sufficient guidance for new content
09:51:40 <@fantasai> florian: The compat spec tends to have a lot less rigor than the mainstream specs have
09:51:48 <@fantasai> florian: It just documents existence of thing, not how it works
09:52:06 <@fantasai> florian: For things that aren't needed, let's not spect them. But things that are necessary for web-compat, then we need to spec them.
09:52:16 <@fantasai> florian: It's a disservice to implementers to not spec them.
09:52:32 <@fantasai> florian: If it wasn't necessary to implement, we wouldn't be speccing at all
09:52:54 <@fantasai> emilio: Regarding fantasai's point, PR just says "legacy features". Open to better suggestions.
09:53:11 <@fantasai> plinss: I'm OK as long as clearly marked as deprecated, as fantasai described
09:53:18 <@fantasai> plinss: Other thoughts?
09:53:47 <@fantasai> RESOLVED: Spec these methods, mark them deprecated, add ADVISEMENT to authors to not teach or use
09:54:14 <@fantasai> florian: When you do this, you can put them at the bottom of the spec, so only ppl who read the whole section notice them
09:54:22 <emilio> ack
09:54:24 <@fantasai> emilio, https://www.w3.org/StyleSheets/TR/2016/readme.html#advisement
09:54:30 <@fantasai> github-bot, end topic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cssom-1 Current Work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants