The ‘@supports’ rule is a conditional group rule whose condition tests whether the user agent supports CSS property:value pairs. http://dev.w3.org/csswg/css3-conditional/#at-supports
Created attachment 141261 [details] Patch CSS is not really in my area of expertise, so excuse any newbie mistakes :-). I suspect this needs more tests, suggestions very welcome.
Implementation aside, I worry about claiming in such explicit terms that we "support" a given property simply because we can parse it.
Sir awesomekling, For Webkit to successfully parse a css property/value pair but not "support" it is a false positive for feature detection and should be avoided. For the most part, vendors to avoid successfully parsing and unsuccessfully implementing, (and Modernizr /HTML5Test/Caniuse/etc all file tickets when they don't) so I feel this dependency is fair.
@Paul: Fair enough, then I have no objections here. :)
Ping? (can anyone suggest relevant reviewers if they're not cc:ed?)
Mozilla has this now: https://bugzilla.mozilla.org/show_bug.cgi?id=649740
Unofficial reports that it's coming to Opera as well: https://twitter.com/frivoal/status/231101124298547200
(In reply to comment #7) > Unofficial reports that it's coming to Opera as well: https://twitter.com/frivoal/status/231101124298547200 Florian works for Opera, so you can see that as a confirmation.
Comment on attachment 141261 [details] Patch Attachment 141261 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/13485882
Comment on attachment 141261 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141261&action=review Informal review. You can ignore the cr-android-ews failure. Should we split up the tests into multiple files? That way, filenames can be used to identify what exactly is being tested, considering in a big list like this (without per-test comments) it may not always be obvious for someone unfamiliar with CSS Conditional Rules. Feel free to hold off on that unless a reviewer feels the same though, as it'd be a lot of work. Ports which don't want this feature (yet) can't disable it right now. Should we add a compile-time feature define, i.e. ENABLE_CSS_CONDITIONAL_RULES? > Source/WebCore/css/CSSGrammar.y:473 > + | supports The specification currently is a young Working Draft, so I think we should prefix this. Mozilla decided not to prefix (https://bugzilla.mozilla.org/show_bug.cgi?id=649740), but it's not entirely obvious to me why. > LayoutTests/css3/supports.html:27 > + @supports not not not not (display: none) { This is incorrect, the declaration following a "not" negation must always be surrounded by parenthesis. > LayoutTests/css3/supports.html:30 > + Maybe add a test for double negation? i.e. @supports not (not (display: block)) {} > LayoutTests/css3/supports.html:122 > + @supports(((((((display: none))))))) { The latest Editor's Draft mentions this syntax for supports_rule: "SUPPORTS_SYM S+ supports_condition group_rule_body" Which implies that one or more space characters need to follow the @supports rule. Mozilla's implementation does not mandate this requirement. Is there some part of the specification I'm missing? > LayoutTests/css3/supports.html:129 > + Some idea for more tests: - Do functions work as property values? - What about functions that'll be evaluated later, i.e. using calc()? Variables are valid values too, but their value is derived from parent rules. I guess that means that it's impossible to test support for properties which values use variables, despite them being valid.
(In reply to comment #10) > (From update of attachment 141261 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=141261&action=review > > Ports which don't want this feature (yet) can't disable it right now. Should we add a compile-time feature define, i.e. ENABLE_CSS_CONDITIONAL_RULES? Can do. > > Source/WebCore/css/CSSGrammar.y:473 > > + | supports > > The specification currently is a young Working Draft, so I think we should prefix this. Mozilla decided not to prefix (https://bugzilla.mozilla.org/show_bug.cgi?id=649740), but it's not entirely obvious to me why. I didn't prefix it because i thought it would defeat the whole purpose, and it also seemed like a simple feature with syntax that was agreed upon by enough people, but i guess you never know with the csswg :P Mozilla has this to say (that i could find): "[*] As prefixing this at-rule has no sense, the @supports at-rule is only supported if the user enables it by setting the config value layout.css.supports-rule.enable to true." From https://developer.mozilla.org/en-US/docs/CSS/@supports I'll ask the opera guys what they're planning. > > LayoutTests/css3/supports.html:27 > > + @supports not not not not (display: none) { > > This is incorrect, the declaration following a "not" negation must always be surrounded by parenthesis. Right, i misread the grammar. > > LayoutTests/css3/supports.html:122 > > + @supports(((((((display: none))))))) { > > The latest Editor's Draft mentions this syntax for supports_rule: > "SUPPORTS_SYM S+ supports_condition group_rule_body" > > Which implies that one or more space characters need to follow the @supports rule. Mozilla's implementation does not mandate this requirement. Is there some part of the specification I'm missing? Everywhere else S* is used, so i think it was just inertia on my part. Maybe the same thing happened to Cameron?. Funnily enough, it doesn't seem like they pass the tests in this patch that don't have whitespace between the operators and the conditions (i.e. #t21 to #t24).
(In reply to comment #11) > (In reply to comment #10) > > > Source/WebCore/css/CSSGrammar.y:473 > > > + | supports > > > > The specification currently is a young Working Draft, so I think we should prefix this. Mozilla decided not to prefix (https://bugzilla.mozilla.org/show_bug.cgi?id=649740), but it's not entirely obvious to me why. > > I didn't prefix it because i thought it would defeat the whole purpose, and it also seemed like a simple feature with syntax that was agreed upon by enough people, but i guess you never know with the csswg :P > > Mozilla has this to say (that i could find): > "[*] As prefixing this at-rule has no sense, the @supports at-rule is only supported if the user enables it by setting the config value layout.css.supports-rule.enable to true." > From https://developer.mozilla.org/en-US/docs/CSS/@supports > > I'll ask the opera guys what they're planning. Please do not prefix it. We can keep it off the stable channel until it's ready, but prefixing would, as you say, completely defeat the purpose. We discussed this a few weeks ago at the CSSWG face-to-face.
(In reply to comment #12) > Please do not prefix it. We can keep it off the stable channel until it's ready, but prefixing would, as you say, completely defeat the purpose. We discussed this a few weeks ago at the CSSWG face-to-face. Ok, agreed. In that case we definitely need a compile-time flag.
Created attachment 160546 [details] Patch
(In reply to comment #14) > Created an attachment (id=160546) [details] > Patch Added the ENABLE flag and a few more tests.
Comment on attachment 160546 [details] Patch Attachment 160546 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13599350 New failing tests: css3/supports.html
Created attachment 160552 [details] Archive of layout-test-results from gce-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment on attachment 160546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160546&action=review > Source/WebCore/css/CSSGrammar.y:663 > ; > > +supports: > + SUPPORTS_SYM maybe_space supports_condition '{' maybe_space block_rule_list save_block { I think it would be good to add feature #if to the grammar too so keep track of everything that belongs to this feature. Bug 94290 has a WIP patch...
(In reply to comment #10) > > LayoutTests/css3/supports.html:122 > > + @supports(((((((display: none))))))) { > > The latest Editor's Draft mentions this syntax for supports_rule: > "SUPPORTS_SYM S+ supports_condition group_rule_body" > > Which implies that one or more space characters need to follow the @supports rule. Mozilla's implementation does not mandate this requirement. Is there some part of the specification I'm missing? Since @media has an S* there (and everyone is interop about it), we've changed @supports to match.
Created attachment 169233 [details] Patch for landing No substantial changes. Rebased, added feature ifdefs in grammar file, skipped the tests in TestExpectation files.
Comment on attachment 169233 [details] Patch for landing Clearing flags on attachment: 169233 Committed r131783: <http://trac.webkit.org/changeset/131783>
Would you add CSS_CONDITIONAL_RULES to http://trac.webkit.org/wiki/FeatureFlags please?
(In reply to comment #22) > Would you add CSS_CONDITIONAL_RULES to http://trac.webkit.org/wiki/FeatureFlags please? Yeah, i'll get to it once i manage to log into trac.
Is there a complementary ticket for supportsCSS()? (Mozilla and Opera completed their implementation with it as well)
(In reply to comment #24) > Is there a complementary ticket for supportsCSS()? > (Mozilla and Opera completed their implementation with it as well) I don't think so, but i was planning on looking at that next week. Feel free to file a bug for it and cc me.
(In reply to comment #25) > (In reply to comment #24) > > Is there a complementary ticket for supportsCSS()? > > (Mozilla and Opera completed their implementation with it as well) > > I don't think so, but i was planning on looking at that next week. Feel free to file a bug for it and cc me. Special very important note! The function was renamed to just "supports()", on a new global CSS object. Check the Editor's Draft for details.
<rdar://problem/18219801>