161747 2016-09-08 11:40:38 -0700 Class selectors on pseudo elements should be forbidden. 2023-12-31 11:42:45 -0800 1 1 1 Unclassified WebKit CSS WebKit Nightly Build Unspecified Unspecified NEW https://bugs.webkit.org/show_bug.cgi?id=241815 P2 Normal --- 1 hyatt webkit-unassigned ahmad.saleem792 annevk ntim oldest_to_newest 1227663 0 hyatt 2016-09-08 11:40:38 -0700 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 1975246 1 ahmad.saleem792 2023-09-03 04:06:59 -0700 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 1975247 2 ahmad.saleem792 2023-09-03 04:08:41 -0700 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? } 1975250 3 ahmad.saleem792 2023-09-03 05:34:24 -0700 (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. 1975307 4 ntim 2023-09-03 21:44:44 -0700 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. 2002351 5 ahmad.saleem792 2023-12-31 09:31:20 -0800 (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? 2002352 6 ahmad.saleem792 2023-12-31 09:32:10 -0800 (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. :-( 2002355 7 annevk 2023-12-31 11:01:30 -0800 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! 2002356 8 ahmad.saleem792 2023-12-31 11:02:26 -0800 (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. 2002357 9 ahmad.saleem792 2023-12-31 11:42:45 -0800 Draft PR - https://github.com/WebKit/WebKit/pull/22277 Any help and guidance welcome!