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

Impossible to satisfy CSS condition #12089

Closed
tmb-github opened this issue Feb 13, 2021 · 8 comments
Closed

Impossible to satisfy CSS condition #12089

tmb-github opened this issue Feb 13, 2021 · 8 comments

Comments

@tmb-github
Copy link

tmb-github commented Feb 13, 2021

Chrome Canary: 90.0.4417.0 (Official Build) canary (64-bit)
OS: Windows 8.1
Lighthouse: 7.0.1

Lighthouse 7.0.1 now issues this deprecation warning when it encounters ::webkit-details-marker:

[Deprecation] ::-webkit-details-marker pseudo element selector is deprecated. Please use ::marker instead. See https://chromestatus.com/feature/6730096436051968 for more details

The info at that link states:

The default value of CSS display property for <summary> is changed to list-item from block. We also support ::marker pseudo element selector for <summary>, and remove ::-webkit-details-marker pseudo element selector. Before this change, developers did the following in order to hide the details marker: summary::-webkit-details-marker { display: none; margin-inline-end: 0; } Now developers can do: summary { display: block; } or summary { list-style-type: none; }

This is great. However, ::marker isn't supported on Safari or Opera (or older versions of Chrome). See support table:

MDN on ::marker

This means that I need to use fallback CSS for browsers who do not support ::marker. This is essential, because I'm suppressing the default triangle marker in <summary> elements in my site. Thus, my cross-browser CSS is:

@supports selector(::marker) {
    .main summary {
        list-style: none;
    }
}

@supports not selector(::marker) {
    .main summary::-webkit-details-marker {
        display: none;
    }
}

However, Canary 90 cannot detect that I'm filtering non-support of ::marker when I use the fallback of ::webkit-details-marker:

[Deprecation] ::-webkit-details-marker pseudo element selector is deprecated. Please use ::marker instead. See https://chromestatus.com/feature/6730096436051968 for more details.

The presence of the fallback CSS lowers my Lighthouse score.

Shouldn't Chromium/Lighthouse "see" that the pseudo-element is being rendered conditionally on support, and thus not issue a deprecation warning? Alternatively, how can I write my CSS to suppress this warning, and therefore not be penalized by Lighthouse?

So, I am using ::marker, but by necessity I'm providing a fallback for non-compliant browsers, and Lighthouse is penalizing me for using the fallback. (I tried posting this issue to this group: Chromium Discuss , but, reason unknown, it wouldn't post.)

Please help!

@tmb-github
Copy link
Author

tmb-github commented Feb 14, 2021

Cross-browser testing reveals inconsistent results. Below is code I tested in Canary 90, Edge 88, Opera 74, and Firefox 85. If summary::marker is supported, confirmation text will appear, and the summary caret should not appear to the left of the first summary. If summary::-webkit-details-marker is supported, then confirmation text will appear, and the summary caret should not appear to the left of the second summary.

Canary shows support for both pseudo elements but only suppress the first summary caret.
Edge 88 shows support for both pseudo elements but only suppresses second summary caret (::-webkit-details-marker),
Opera 74 shows support for both pseudo elements but only suppresses second summary caret (::-webkit-details-marker).
Firefox only shows support for summary::marker and only suppresses the first summary caret.

Firefox is the only browser that behaves as expected.

This may not be the best way to test this feature. Clearly, we need to use ::-webkit-details-marker in fallback CSS. The larger problem is that Lighthouse reduces our score if we use the fallback CSS.

Here is the HTML used for the test described above:

<!doctype html>
<html lang=en-us>
<head>
    <title>Summary Marker Suppression Test</title>
    <meta charset=utf-8>
    <style>

    .marker p, 
    .webkit-details-marker p {
        display: none;
    }

    @supports selector(summary::marker) {
        .marker p {
            display: block;
        }
        .marker summary {
            display: block;
            list-style: none;
        }
    }

    @supports selector(summary::-webkit-details-marker) {
        .webkit-details-marker p {
            display: block;
        }
        .webkit-details-marker summary::-webkit-details-marker {
            display: none;
            margin-inline-end: 0;
        }
    }

    </style>
</head>

<body>
    <div class=marker>
        <p>summary::marker is supported</p> 
        <details>
            <summary>SUMMARY: If no marker to the left, it's suppressed with NEW CSS</summary>
            Summary Text
        </details>
    </div>

    <div class=webkit-details-marker>
        <p>summary::-webkit-details-marker is supported</p>
        <details>
            <summary>SUMMARY: If no marker to the left, it's suppressed with ::-webkit-details-marker</summary>
            Summary Text
        </details>
    </div>

</body>

</html>

@adstr123
Copy link

I am also encountering this issue which is resulting in a hit to my Best Practises score.

@connorjclark
Copy link
Collaborator

So, I am using ::marker, but by necessity I'm providing a fallback for non-compliant browsers, and Lighthouse is penalizing me for using the fallback. (I tried posting this issue to this group: Chromium Discuss , but, reason unknown, it wouldn't post.)

This seems like an issue with Chrome being to aggressive in emitting a deprecation. I'd expect the CSS you have to not trigger this, since it's behind a @supports declaration. Can you file an issue here? https://bugs.chromium.org/p/chromium/issues/list

In the meantime, we can consider ignoring this deprecation in Lighthouse.

@tmb-github
Copy link
Author

I've now logged it here: https://bugs.chromium.org/p/chromium/issues/detail?id=1189809

@connorjclark
Copy link
Collaborator

connorjclark commented Mar 19, 2021

FYI: Safari doesn't support @supports selector(...) yet :( https://bugs.webkit.org/show_bug.cgi?id=199237 it only just landed and as far as I can tell it hasn't hit stable Safari.

@tmb-github
Copy link
Author

Yikes! Sorry about that oversight. That actually complicates matters further, because it's not possible to wrap the use of ::-webkit-details-marker in a @supports block to provide compatibility to browsers that require the old method. And by my new test, older Chrome (I tested 75) and current Opera (74) and Safari (14) all need the fallback CSS.

Try this revised code in older and current Chrome, Firefox, Edge, Opera, and Safari and see;

<!doctype html>
<html lang=en-us>
<head>
    <title>Summary Marker Suppression Test</title>
    <meta charset=utf-8>
    <style>

    .marker p, 
    .webkit-details-marker p {
        display: none;
    }

/* For browsers that support ::marker (Chrome, Edge, Firefox) */
    .marker p {
        display: block;
    }
    .marker summary {
        display: block;
        list-style: none;
    }

/* For browsers that do not support ::marker and require ::-webkit-details-marker (older Chrome, Safari, Opera) */
    .webkit-details-marker p {
        display: block;
    }
    .webkit-details-marker summary::-webkit-details-marker {
        display: none;
        margin-inline-end: 0;
    }

    </style>
</head>

<body>
    <h1></h1>
    <div class=marker>
        <p>summary::marker is supported</p> 
        <details><summary>SUMMARY: If no marker to the left, it's suppressed with NEW CSS</summary>Summary Text</details>
    </div>

    <div class=webkit-details-marker>
        <p>summary::-webkit-details-marker is supported</p>
        <details><summary>SUMMARY: If no marker to the left, it's suppressed with ::-webkit-details-marker</summary>Summary Text</details>
    </div>

<script>
function browserAndVersion () {
    var ua = navigator.userAgent;
    var tem;
    var M = ua.match(/(opera|chrome|safari|firefox|msie|trident|edg|edge(?=\/))\/?\s*(\d+)/i) || [];
    if (/trident/i.test(M[1])) {
        tem = /\brv[\u0020:]+(\d+)/g.exec(ua) || [];
        return 'IE ' + (tem[1] || '');
    }
    if (M[1] === 'Chrome') {
        tem = ua.match(/\b(OPR|Edg)\/(\d+)/);
        if (tem !== null) {
            return tem.slice(1).join(' ').replace('OPR', 'Opera').replace('Edg', 'Edge');
        }
    }
    M = (
        M[2]
        ? [M[1], M[2]]
        : [navigator.appName, navigator.appVersion, '-?']
    );
    tem = ua.match(/version\/(\d+)/i);
    if (tem !== null) {
        M.splice(1, 1, tem[1]);
    }
    return M.join(' ');
}
document.querySelector('H1').innerHTML = browserAndVersion();
</script>
</body>
</html>

@connorjclark
Copy link
Collaborator

The deprecation was reverted in Chrome, so this shouldn't be an issue in Chrome Canary anymore. It will remain in M90 (Apr 13) but be gone in M91 (May 25). We can't do anything about the false-warning for M90.

We might consider blocking the warning in the next LH release, so at least the CLI and PSI will ignore this.

@web-apply
Copy link

thanks

@GoogleChrome GoogleChrome deleted a comment from alfodei22 Nov 8, 2021
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