Bug 77838 – Unknown pseudo-elements do not cause a rule to be dropped

• Component: CSS
• Status: RESOLVED CONFIGURATION CHANGED

[Boris Zbarsky]

Created attachment 125533 [details]
Testcase

The attached testcase renders correctly in Trident, Gecko, and Presto, but not WebKit.

[Alexis Menard (darktears)]

Created attachment 130465 [details]
Patch

[Tony Chang]

Comment on attachment 130465 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130465&action=review

> LayoutTests/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html:6
> +      ::Thisisaboguspseudoelement, body { color: red }

Nit: It would be nice to test : as well, even though we already pass for that case.

[Alexis Menard (darktears)]

Created attachment 130488 [details]
Patch for landing

[Adam Barth]

Comment on attachment 130488 [details]
Patch for landing

This patch makes many tests time out.

[WebKit Review Bot]

Comment on attachment 130488 [details]
Patch for landing

Attachment 130488 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11881034

New failing tests:
fast/dom/HTMLMeterElement/meter-styles-changing-pseudo.html
fast/dom/HTMLMeterElement/meter-writing-mode.html
fast/css/unknown-pseudo-element-matching.html
fast/dom/HTMLMeterElement/meter-element-repaint-on-update-value.html
editing/selection/4397952.html
fast/dom/HTMLMeterElement/meter-appearances-capacity.html
http/tests/security/isolatedWorld/userGestureEvents.html
http/tests/inspector/inspect-element.html
accessibility/aria-disabled.html
fast/css/css-set-selector-text.html
fast/dom/HTMLMeterElement/meter-element.html
fast/css/margin-top-bottom-dynamic.html
css2.1/20110323/abspos-containing-block-initial-004d.htm
fast/dom/HTMLProgressElement/progress-bar-value-pseudo-element.html
fast/dom/HTMLTextAreaElement/reset-textarea.html
fast/dom/HTMLInputElement/input-slider-update.html
fast/css/continuationCrash.html
css2.1/20110323/abspos-containing-block-initial-004b.htm
fast/dom/HTMLProgressElement/progress-writing-mode.html
fast/dom/HTMLTableColElement/resize-table-using-col-width.html
fast/dom/HTMLInputElement/input-slider-update-styled.html
fast/dom/HTMLMeterElement/meter-styles.html
fast/dom/HTMLMeterElement/meter-optimums.html
fast/css/button-height.html
fast/dom/HTMLMeterElement/meter-appearances-rating-relevancy.html
fast/css/rtl-ordering.html
editing/selection/5240265.html
fast/dom/HTMLMeterElement/meter-boundary-values.html
fast/css/css-selector-text.html
fast/block/float/float-avoidance.html

[Alexis Menard (darktears)]

(In reply to comment #5)
> (From update of attachment 130488 [details])
> Attachment 130488 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/11881034
> 
> New failing tests:
> fast/dom/HTMLMeterElement/meter-styles-changing-pseudo.html
> fast/dom/HTMLMeterElement/meter-writing-mode.html
> fast/css/unknown-pseudo-element-matching.html
> fast/dom/HTMLMeterElement/meter-element-repaint-on-update-value.html
> editing/selection/4397952.html
> fast/dom/HTMLMeterElement/meter-appearances-capacity.html
> http/tests/security/isolatedWorld/userGestureEvents.html
> http/tests/inspector/inspect-element.html
> accessibility/aria-disabled.html
> fast/css/css-set-selector-text.html
> fast/dom/HTMLMeterElement/meter-element.html
> fast/css/margin-top-bottom-dynamic.html
> css2.1/20110323/abspos-containing-block-initial-004d.htm
> fast/dom/HTMLProgressElement/progress-bar-value-pseudo-element.html
> fast/dom/HTMLTextAreaElement/reset-textarea.html
> fast/dom/HTMLInputElement/input-slider-update.html
> fast/css/continuationCrash.html
> css2.1/20110323/abspos-containing-block-initial-004b.htm
> fast/dom/HTMLProgressElement/progress-writing-mode.html
> fast/dom/HTMLTableColElement/resize-table-using-col-width.html
> fast/dom/HTMLInputElement/input-slider-update-styled.html
> fast/dom/HTMLMeterElement/meter-styles.html
> fast/dom/HTMLMeterElement/meter-optimums.html
> fast/css/button-height.html
> fast/dom/HTMLMeterElement/meter-appearances-rating-relevancy.html
> fast/css/rtl-ordering.html
> editing/selection/5240265.html
> fast/dom/HTMLMeterElement/meter-boundary-values.html
> fast/css/css-selector-text.html
> fast/block/float/float-avoidance.html

I tracked down those failure and I know why they happen.

The problem with my patch is that it filters away all unknown pseudo elements. While it's fine for the test case I have added it unfortunately fires back when it comes to webkit specific pseudo elements. We find many of them in html.css (e.g ::-webkit-textfield-decoration-container, ::-webkit-validation-bubble, ...). Those are treated as unknown pseudo elements so they are filtered away and no rules are created which explains the failures. CSSSelector.h contains the list of valid pseudo elements and those are not part of this list.

One approach could be to make those pseudo element not unknown by supporting them in CSSSelector.h, the problem is that they seem to be added a bit randomly  and I'm not sure we want to pollute the enum of CSSSelector with lot of values.

One hack will be to create an additional enum value PseudoWebKit that will be set to true whenever the string of the pseudo element is starting with -webkit-*. It will solve the use case of the reporter by filtering away everything that doesn't exist (::foobar) but we still not protect this case ::-webkit-FooBar. It will also fix the layout tests failure by letting through all webkit specific declaration.

Any thought? Better ideas?

[Erik Arvidsson]

I believe we should try to get the spec changed here. With this change the following would not work:

#test, ::-webkit-scrollbar {
  background: red;
}

and indeed this breaks in all but webkit.

http://plexode.com/eval3/#ht=%3Cstyle%3E%0A%23test%2C%20%3A%3A-webkit-scrollbar%20{%0A%20%20background%3A%20red%3B%0A}%0A%0A%23test%20{%0A%20%20width%3A%20100px%3B%0A%20%20height%3A%20100px%3B%0A%20%20overflow%3A%20scroll%3B%0A}%0A%3C%2Fstyle%3E%0A%0A%3Cdiv%20id%3Dtest%3E%3C%2Fdiv%3E&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1

I think the right behavior would be to allow unrecognized pseudo classes and unrecognized pseudo elements

[Dimitri Glazkov (Google)]

(In reply to comment #7)
> I believe we should try to get the spec changed here. With this change the following would not work:
> 
> #test, ::-webkit-scrollbar {
>   background: red;
> }
> 
> and indeed this breaks in all but webkit.
> 
> http://plexode.com/eval3/#ht=%3Cstyle%3E%0A%23test%2C%20%3A%3A-webkit-scrollbar%20{%0A%20%20background%3A%20red%3B%0A}%0A%0A%23test%20{%0A%20%20width%3A%20100px%3B%0A%20%20height%3A%20100px%3B%0A%20%20overflow%3A%20scroll%3B%0A}%0A%3C%2Fstyle%3E%0A%0A%3Cdiv%20id%3Dtest%3E%3C%2Fdiv%3E&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1
> 
> I think the right behavior would be to allow unrecognized pseudo classes and unrecognized pseudo elements

I actually intentionally relaxed this in WebKit (https://bugs.webkit.org/show_bug.cgi?id=46595), so that we can use pseudoelements in shadow DOM. I am ambivalent on whether using pseudoelement syntax is the right approach here, but it is already present in WebKit.

[Boris Zbarsky]

> I believe we should try to get the spec changed here

It might be a bit of a hard sell given that everyone except WebKit implements the spec correctly and specifying any other behavior is nontrivial (because unknown pseudo-elements can have all sorts of arbitrary syntax).

[Ahmad Saleem]

Created attachment 460306 [details]
Safari 15.5 matches other browsers

I am not able to reproduce the following bug in Safari 15.5 on macOS 12.4 and it matches all other browser as shown in the attached screenshots.

If I am testing incorrectly, please retest accordingly. Else if it was something fixed along the line, can it be marked as "RESOLVED CONFIGURATION CHANGED" or "RESOLVED INVALID"? Thanks!

[Ryosuke Niwa]

Yeah, I'm pretty sure we fixed this by adopting Blink's new CSS parser.