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

[Feature] Compat-API should not flag issues if code is in @supports [3] #1869

Closed
molant opened this issue Feb 8, 2019 · 10 comments · Fixed by #2007
Closed

[Feature] Compat-API should not flag issues if code is in @supports [3] #1869

molant opened this issue Feb 8, 2019 · 10 comments · Fixed by #2007
Assignees
Milestone

Comments

@molant
Copy link
Member

molant commented Feb 8, 2019

🚀 Feature request

Description

Right now the compat-api for CSS doesn't check if a feature is being featured detected previously via @supports and thus causing false positives.

The hint should be aware of those.

What scenarios will this solve?

Flagging false positives when users run the compat.

@molant molant added this to the 1902-2 milestone Feb 11, 2019
@sarvaje sarvaje self-assigned this Feb 15, 2019
@sarvaje
Copy link
Contributor

sarvaje commented Feb 16, 2019

@webhintio/core I think I need a little bit more explanation here. Do we want to ignore completely the @support block?
If we have a CSS like this, what should the hint check?:

@supports (display: flex) {
    .content {
        display: flex;
        align-items: center;
        justify-content: center;
    }
}

html,
body {
    padding: 0;
    margin: 0;
    height: 100%;
    width: 100%;
}

.content {
    background-color: #777;
    color: #000;
    height: 100%;
    width: 100%;
}

p {
    margin: 0;
}

Also, what happen when a browser doesn't support @supports should the hint ignore it?

@molant
Copy link
Member Author

molant commented Feb 16, 2019

I think the idea is:

  • If a browser doesn't support a property:
    • If it supports @supports check if it is in a block that checks for it
    • If it doesn't support @supports, ignore that block entirely

So with your flexbox example and looking into the MDN data:

  • IE10 shouldn't show any problem even though it only supports the prefixed version because it doesn't have @supports

A better example will be grid:

  • Edge 13+ has @supports
  • Edge 16+ has grid, previous versions had only the prefixed version

If someone did something attrocious like:

@supports (display: flex) {
    .content {
        display: grid;
        ...
    }
}

This should be a problem in Edge 15 and lower but not 16.

@sarvaje
Copy link
Contributor

sarvaje commented Feb 19, 2019

So basically,

if @support is not supported, ignore the entire block.
if @support is supported, analyze what is inside.

right?

@molant
Copy link
Member Author

molant commented Feb 19, 2019

if @support is supported, analyze what is inside.

More like

if @support is supported, check browser support of the feature and then analyze what's inside

@sarvaje
Copy link
Contributor

sarvaje commented Feb 19, 2019

if @support is supported, check browser support of the feature and then analyze what's inside

What do you mean with check browser support of the feature?

@molant
Copy link
Member Author

molant commented Feb 20, 2019

With the following example and for the case of Edge 13:

@supports (display: grid) {
  // ...
}

Edge 13 implements @supports and does not implement grid so the code in the block should be ignored.

Hope it is clearer now.

@sarvaje
Copy link
Contributor

sarvaje commented Feb 20, 2019

Ahh yes, now it is clearer :)

@molant molant modified the milestones: 1902-2, 1903-1 Feb 25, 2019
@sarvaje sarvaje self-assigned this Mar 1, 2019
@sarvaje
Copy link
Contributor

sarvaje commented Mar 4, 2019

@molant I have a question. What if the CSS contains something like this and we are on Edge 13?:

@support not (display: grid) {
    // ...
}

Should we ignore the @support block or not?

@molant
Copy link
Member Author

molant commented Mar 4, 2019

Don't think we should ignore it.
Edge 13 has @supports and the condition checks so we should analyze the contents.

@sarvaje
Copy link
Contributor

sarvaje commented Mar 4, 2019

ok!

@antross antross modified the milestones: 1903-1, 1903-2 Mar 8, 2019
@molant molant changed the title [Feature] Compat-API should not flag issues if code is in @supports [Feature] Compat-API should not flag issues if code is in @supports [3] Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants