-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add CSSGroupingRule properties #11631
Conversation
"spec_url": "https://drafts.csswg.org/cssom/#dom-cssgroupingrule-cssrules", | ||
"support": { | ||
"chrome": { | ||
"version_added": "45" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be correct, but due to #7844 I'm suspicious of any Chrome 45 versions. What testing was done to get this version? There are generated tests for this in mdn-bcd-collector, but they were broken. I've sent foolip/mdn-bcd-collector#1323 to fix it and then it should be possible to use these tests:
https://staging-dot-mdn-bcd-collector.appspot.com/tests/api/CSSGroupingRule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied the data from the main interface (that I haven't changed), after searching to see if there was any indication that this had been partly implemented. So if it's wrong, the main interface is likely wrong too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this came from https://bugs.chromium.org/p/chromium/issues/detail?id=496381, and the date does line up with Chrome 45. I've confirmed that the CSSGroupingRule
is there in Chrome 45, but not in 44.
That just shuffled the prototype chain around a little bit and is a case of https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines.md#apis-moved-on-the-prototype-chain, but sorting that out need not block this PR. I'll review.
"version_added": "32" | ||
}, | ||
"safari": { | ||
"version_added": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Safari does have CSSGroupingRule
, but the parent feature is wrong too. I've filed #11644.
CSSGroupingRule was missing properties, this PR adds them and their specdata https://drafts.csswg.org/cssom/#cssgroupingrule