Bug 77838 - Unknown pseudo-elements do not cause a rule to be dropped
: Unknown pseudo-elements do not cause a rule to be dropped
Status: ASSIGNED
: WebKit
CSS
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2012-02-05 12:54 PST by
Modified: 2012-03-11 09:02 PST (History)


Attachments
Testcase (186 bytes, text/html)
2012-02-05 12:54 PST, Boris Zbarsky
no flags Details
Patch (7.13 KB, patch)
2012-03-06 16:26 PST, Alexis Menard (darktears)
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (6.99 KB, patch)
2012-03-06 17:33 PST, Alexis Menard (darktears)
abarth: commit‑queue-
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-02-05 12:54:42 PST
Created an attachment (id=125533) [details]
Testcase

The attached testcase renders correctly in Trident, Gecko, and Presto, but not WebKit.
------- Comment #1 From 2012-03-06 16:26:44 PST -------
Created an attachment (id=130465) [details]
Patch
------- Comment #2 From 2012-03-06 16:51:15 PST -------
(From update of attachment 130465 [details])
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.
------- Comment #3 From 2012-03-06 17:33:38 PST -------
Created an attachment (id=130488) [details]
Patch for landing
------- Comment #4 From 2012-03-07 09:52:01 PST -------
(From update of attachment 130488 [details])
This patch makes many tests time out.
------- Comment #5 From 2012-03-08 09:46:27 PST -------
(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
------- Comment #6 From 2012-03-08 10:02:10 PST -------
(In reply to comment #5)
> (From update of attachment 130488 [details] [details])
> Attachment 130488 [details] [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?
------- Comment #7 From 2012-03-08 10:42:05 PST -------
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
------- Comment #8 From 2012-03-08 10:56:15 PST -------
(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.
------- Comment #9 From 2012-03-08 20:39:00 PST -------
> 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).