Skip to content
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

[css-conditional-3] Are there issues with !important in @supports? (Is the grammar wrong?) #5559

Closed
svgeesus opened this issue Sep 29, 2020 · 7 comments

Comments

@svgeesus
Copy link
Contributor

@SimonSapin [wrote on www-style in 2013](

Le 30/04/2013 07:44, John Daggett a écrit :

I've updated the CSS3 Fonts draft based on Simon's feedback.

AFAICT, the point of <descriptor_declaration> is to not have the
production for !important. You could, like @supports, refer to
defined in the Core Grammar instead.
Does @supports specifically ban the production? I don't see
the language in there for that. It's a relatively minor point but I
added the production to be clear that '!important' has no function in
@font-face rules (there have been issues brought up in the past in
relation to this).

I just assumed that because @supports defines explicitly
by referring to Chapter 4:

declaration : property S* ':' S* value;

rather than Appendix G:

declaration
: property ':' S* expr prio?
;

But I was wrong. !important is allowed and has no effect. This is
specified in section 6, just before section 6.1:

A trailing ‘!important’ on a declaration being tested is allowed,
though it won't change the validity of the declaration.

For example, the following rule is valid:

@supports (display: flexbox !important) {
// ...
}

@svgeesus
Copy link
Contributor Author

@SimonSapin a few questions:

  1. Is this just about @supports or is it about !important in descriptors in general
  2. Does the point you made by referring to CSS2.1 grammar still hold now that Conditional 3 uses the CSS Syntax module instead?
  3. Are you saying that
  • !important in descriptors is allowed, valuable, and we should test it
  • !important in descriptors is allowed, unfortunately, and we change the grammar to disalow it
  • something else?

@tabatkins
Copy link
Member

Just to confirm, here's a testcase

Chrome and Firefox both show green (!important is allowed)

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Oct 22, 2020

The CSS Working Group just discussed Are there issues with !important in @supports? (Is the grammar wrong?).

RESOLVED: Closed - no change
RESOLVED: Publish a CRD of CSS Conditional L3

The full IRC log of that discussion <gregwhitworth> Topic: Are there issues with !important in @supports? (Is the grammar wrong?)
<astearns> github: https://github.com//issues/5559
<TabAtkins> github: https://github.com//issues/5559
<gregwhitworth> TabAtkins: this is going to be similar to the first one
<gregwhitworth> TabAtkins: syntax currently allows a bang at the end, it has no impact on the result
<gregwhitworth> TabAtkins: implementations that I tested ignore it consistently. Should this be confirmed that we're fine with or not?
<gregwhitworth> TabAtkins: Simon found it unfortunate possibly in the past? I feel we're going to lean towards consistency
<gregwhitworth> florian: so you can include it?
<gregwhitworth> TabAtkins: yes Chrome/FF do, just ignore it
<gregwhitworth> florian: that's good, we may have something later that people can check for that
<gregwhitworth> Proposed Resolution: Closed - no change
<gregwhitworth> Resolved: Closed - no change
<bkardell_> WebKit also seems green in the test
<gregwhitworth> chris: we're at 0 open issues, that means we'll be able to publish a CR thing outside of privacy and i18n
<gregwhitworth> astearns: deadline?
<gregwhitworth> chris: no
<gregwhitworth> fantasai: we can publish a crd anytime we want
<gregwhitworth> astearns: are there other changes?
<gregwhitworth> chris: every edit since 2013
<gregwhitworth> astearns: on some that's a short list :P
<gregwhitworth> astearns: should we have a crd to help others
<gregwhitworth> fantasai: I think it's a good idea
<gregwhitworth> Proposed Resolution: Publish a CRD of CSS Conditional L3
<gregwhitworth> Resolved: Publish a CRD of CSS Conditional L3

@Loirooriol
Copy link
Contributor

So is this expected?

CSS.supports("background: green !important"); // true
CSS.supports("background", "green !important"); // false

@fantasai
Copy link
Collaborator

fantasai commented Nov 3, 2020

@Loirooriol Mind filing that as a separate issue? Afaict the spec isn't particularly clear...

@svgeesus
Copy link
Contributor Author

svgeesus commented Nov 3, 2020

@fantasai Closed - no change so which edits?

@fantasai
Copy link
Collaborator

fantasai commented Nov 4, 2020

@svgeesus Idk, maybe I was looking at the wrong issue or something. :)

@fantasai fantasai closed this as completed Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants