Bug 98836 – Generated should not be supported for things with a shadow

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Elliott Sprehn]

As far as CSS is concerned :before and :after don't apply to things like <input> because they're opaque replaced content. At one point there was code to prevent this, but the new Shadow DOM based implementations of some input types started supporting it but shouldn't have (yet). It looks like we only supported it for inputs of type="time" / month, week, date anyway.

The new generated content implementation doesn't support generated content on things with shadows (yet) so these tests now fail. Until we know what to do with generated content when you have a shadow we should just hide it when you have a shadow.

See:

fast/forms/time-multiple-fields/time-multiple-fields-appearance-pseudo-elements.html
fast/forms/month-multiple-fields/month-multiple-fields-appearance-pseudo-elements.html
fast/forms/week-multiple-fields/week-multiple-fields-appearance-pseudo-elements.html
fast/forms/date-multiple-fields/date-multiple-fields-appearance-pseudo-elements.html

[Elliott Sprehn]

Created attachment 169495 [details]
Patch

[WebKit Review Bot]

Comment on attachment 169495 [details]
Patch

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

New failing tests:
fast/forms/pseudo-elements.html

[Elliott Sprehn]

(In reply to comment #2)
> (From update of attachment 169495 [details])
> Attachment 169495 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/14459272
> 
> New failing tests:
> fast/forms/pseudo-elements.html

Hmm, this doesn't fail locally. I'll build it on cr-linux and try to run the tests. We should figure out how to make it email you the failing test output too.

[Elliott Sprehn]

Created attachment 169703 [details]
Patch

[Elliott Sprehn]

(In reply to comment #4)
> Created an attachment (id=169703) [details]
> Patch

I figured out the test failures. We were missing a canHaveGeneratedChildren() { return false; } on RenderListBox which seems to be just an oversight. We already have it on RenderMenuList. The only reason this appears to pass on Mac is because the generated content is above the first multi select you can't see it.

[Kent Tamura]

Comment on attachment 169703 [details]
Patch

The behavior changes for RenderListBox and multiple-fields input elements is ok.  However I'm not sure if we should disable generated contents for shadow hots at all.

[Elliott Sprehn]

(In reply to comment #6)
> (From update of attachment 169703 [details])
> The behavior changes for RenderListBox and multiple-fields input elements is ok.  However I'm not sure if we should disable generated contents for shadow hots at all.

There's still debate about how :before and :after should work with shadow. For example Tab was saying we should make <content> select them and project them into the shadow and that's really hard with the current implementation, but will be much easier with the new implementation.

I'd really like to get this landed as a temporary measure so people don't depend on our unspecified behavior which might have to change and because the new generated content implementation is going to temporarily disable support. Getting the new generated content landed is really important because it removes a lot of complexity, fixes lots of bugs, and gives us support for things like animating pseudos.

Once the new generated content implementation in Bug 98836 is relanded we can add support back. I certainly don't think we should disable support forever. Does that work for you? :)

[Elliott Sprehn]

(In reply to comment #7)
> (In reply to comment #6)
> ...
> Once the new generated content implementation in Bug 98836 is relanded

Sorry that's Bug 95117

[Elliott Sprehn]

@dglazgov @morrita Does the above plan work for you? This blocks the new generated content implementation that should make our lives easier after it lands. :) Then we can bring support back for :before and :after on shadows after Tab/Dimitri updates the spec.

[Hajime Morrita]

(In reply to comment #9)
> @dglazgov @morrita Does the above plan work for you? This blocks the new generated content implementation that should make our lives easier after it lands. :) Then we can bring support back for :before and :after on shadows after Tab/Dimitri updates the spec.

The plan sounds reasonable. Could you file a bug for get shadow+generated back?
I'm not sure how we can support new-style generated contents for element with shadow.
We need to figure out the way after Bug 95117 is landed.

[Dimitri Glazkov (Google)]

(In reply to comment #10)
> (In reply to comment #9)
> > @dglazgov @morrita Does the above plan work for you? This blocks the new generated content implementation that should make our lives easier after it lands. :) Then we can bring support back for :before and :after on shadows after Tab/Dimitri updates the spec.
> 
> The plan sounds reasonable. Could you file a bug for get shadow+generated back?
> I'm not sure how we can support new-style generated contents for element with shadow.
> We need to figure out the way after Bug 95117 is landed.

SGTM.

[Dimitri Glazkov (Google)]

Comment on attachment 169703 [details]
Patch

The pills are all purple. Can haz green pills before land?

[Elliott Sprehn]

Created attachment 170216 [details]
Patch for landing

[Elliott Sprehn]

Created attachment 170233 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 170233 [details]
Patch for landing

Clearing flags on attachment: 170233

Committed r132269: <http://trac.webkit.org/changeset/132269>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Elliott Sprehn]

This broke the checkboxes and radio buttons in Chrome's settings page http://code.google.com/p/chromium/issues/detail?id=157664

It also appears that people have used the radio button/checkbox hack elsewhere:
http://www.inserthtml.com/2012/06/custom-form-radio-checkbox/
http://www.red-team-design.com/css-generated-content-replaced-elements

Mozilla has said they have no plans to implement it though because the spec said we shouldn't have supported it anyway, and it appears WebKit is the only browser that supports it (by accident too):
https://bugzilla.mozilla.org/show_bug.cgi?id=241985

This raises the question, should we add back support for checkboxes and radio buttons because fixing this bug on form controls could possibly break some WebKit specific apps (maybe iOS/Android only web apps)?

[Elliott Sprehn]

(In reply to comment #17)
> ...
> 
> This raises the question, should we add back support for checkboxes and radio buttons because fixing this bug on form controls could possibly break some WebKit specific apps (maybe iOS/Android only web apps)?

Awesomely this also broke a demo from Eric Bidelman:

http://ericbidelman.tumblr.com/post/23615290220/data-binding-using-data-attributes

and the cool UI demo from a few months back:

http://lab.simurai.com/umbrui/

It's also notable that in all the comments on these pages lots of people mention the WebKit behavior as being a bug, ex. Lea Verou's comment and many others on the red team design page, or the first comment on Eric's blog post.

I'm leaning towards leaving this fix in, but it is unfortunate people started messing with it in the past year.

[Hajime Morrita]

I believe certain number of mobile websites relies on this.

When I made this available for <input> by accident, some people took it as a good thing.
It looks people are aware of this kind of problem and hacks it intentionally.
That's so unfortunate. But we need to make it work since there is no easy replacement
unlike API unprefixing.

[Kent Tamura]

Why did this affect checkbox and radio? They don't have shadow trees, right?

[Dimitri Glazkov (Google)]

Argh. This sucks. Typically, if there's a content in the wild that uses non-standard, under-specified feature, we WebKit peeps are stuck with supporting it. Don't break the web and that sort of thing.

[Tab Atkins]

(In reply to comment #19)
> I believe certain number of mobile websites relies on this.
> 
> When I made this available for <input> by accident, some people took it as a good thing.
> It looks people are aware of this kind of problem and hacks it intentionally.
> That's so unfortunate. But we need to make it work since there is no easy replacement
> unlike API unprefixing.

I'd like us to try just breaking those sites first, before we unilaterally decide we're stuck with it.  It appears that lots of authors *know* that it's WebKit-specific and technically a bug.

Do we know what kind of sites/numbers we're looking at?

[Eric Seidel (no email)]

I would agree.  Not breaking it seems to be implicitly asking other engines to eventually add this quirk.

[WebKit Review Bot]

Re-opened since this is blocked by bug 100412

[Vincent Scheib]

Created attachment 171011 [details]
Patch for re-landing

[Vincent Scheib]

Re-landed as http://trac.webkit.org/changeset/132696, will likely need baseline cleanup which I will monitor for.

[WebKit Review Bot]

Re-opened since this is blocked by bug 100565

[Elliott Sprehn]

This was rolled out again because it breaks all the radio buttons and checkboxes in Chrome. We need a new patch that allows generated content on checkboxes and radio buttons with -webkit-appearance: none and also has this.