Bug 161747 – Class selectors on pseudo elements should be forbidden.

• Status: NEW
• Component: CSS

[Dave Hyatt]

From the corresponding Chromium bug:

We have this selector in mediaControls(New).css:

video::-webkit-media-text-track-region-container.scrolling

Only user action pseudo classes should be allowed after pseudo elements. We should rewrite this somehow. If we need to express this with custom pseudo elements, an internal :scrolling pseudo class would have been better, I think.

https://bugs.chromium.org/p/chromium/issues/detail?id=578131

[Ahmad Saleem]

We still have this FIXME:

https://searchfox.org/wubkat/source/Source/WebCore/css/parser/CSSSelectorParser.cpp#618

Plus this FIXME was removed by Blink in 2019: https://chromium.googlesource.com/chromium/src.git/+/6c24c2e8b1963818eabe2038a0b74db2e86de83f

[Ahmad Saleem]

Plus for some reason, we don't have 'return nulltpr' in this:


    if (m_precedingPseudoElement) {
        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=161747
        // The UASheetMode check is a work-around to allow this selector in mediaControls(New).css:
        // video::-webkit-media-text-track-region-container.scrolling
        if (m_context.mode != UASheetMode && !isSimpleSelectorValidAfterPseudoElement(*selector, *m_precedingPseudoElement))
            m_failedParsing = true;
// HERE MISSING Return?
    }

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #2)
> Plus for some reason, we don't have 'return nulltpr' in this:
> 
> 
>     if (m_precedingPseudoElement) {
>         // FIXME: https://bugs.webkit.org/show_bug.cgi?id=161747
>         // The UASheetMode check is a work-around to allow this selector in
> mediaControls(New).css:
>         // video::-webkit-media-text-track-region-container.scrolling
>         if (m_context.mode != UASheetMode &&
> !isSimpleSelectorValidAfterPseudoElement(*selector,
> *m_precedingPseudoElement))
>             m_failedParsing = true;
> // HERE MISSING Return?
>     }

Adding 'return nullptr;' compiles - haven't run any test.

[Tim Nguyen (:ntim)]

If we were to fix this, `m_context.mode != UASheetMode &&` needs to be removed. Though it does look like we still rely on this internally: `-webkit-media-text-track-region-container.scrolling` still has hits internally.

[Ahmad Saleem]

(In reply to Tim Nguyen (:ntim) from comment #4)
> If we were to fix this, `m_context.mode != UASheetMode &&` needs to be
> removed. Though it does look like we still rely on this internally:
> `-webkit-media-text-track-region-container.scrolling` still has hits
> internally.

-webkit-media-text-track-region-container.scrolling <- hit is in 'mediaControls.css', which Anne is planning to remove this with:

https://github.com/WebKit/WebKit/pull/22265

So I think we can fix this now?

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #5)
> (In reply to Tim Nguyen (:ntim) from comment #4)
> > If we were to fix this, `m_context.mode != UASheetMode &&` needs to be
> > removed. Though it does look like we still rely on this internally:
> > `-webkit-media-text-track-region-container.scrolling` still has hits
> > internally.
> 
> -webkit-media-text-track-region-container.scrolling <- hit is in
> 'mediaControls.css', which Anne is planning to remove this with:
> 
> https://github.com/WebKit/WebKit/pull/22265
> 
> So I think we can fix this now?

Ignore. Anne is removing 'mediaControlsApple.css'. I mixed it. :-(

[Anne van Kesteren]

Ahmad, I think we can use 

video[pseudo="-webkit-media-text-track-region-container.scrolling"].scrolling

here. We have used [pseudo] elsewhere to work around similar issues. Would you be willing to give that a go?

Also, happy new year!

[Ahmad Saleem]

(In reply to Anne van Kesteren from comment #7)
> Ahmad, I think we can use 
> 
> video[pseudo="-webkit-media-text-track-region-container.scrolling"].scrolling
> 
> here. We have used [pseudo] elsewhere to work around similar issues. Would
> you be willing to give that a go?
> 
> Also, happy new year!

Happy New Year! Should I update 'mediaControls.css' to above. Happy to give it a go.

[Ahmad Saleem]

Draft PR - https://github.com/WebKit/WebKit/pull/22277

Any help and guidance welcome!