Bug 104855 - Block level pseudo elements bleed background color to <html>
Summary: Block level pseudo elements bleed background color to <html>
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Elliott Sprehn
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-12 15:39 PST by Elliott Sprehn
Modified: 2012-12-20 10:49 PST (History)
5 users (show)

See Also:


Attachments
Patch (2.54 KB, patch)
2012-12-12 15:55 PST, Elliott Sprehn
no flags Details | Formatted Diff | Diff
Patch (2.95 KB, patch)
2012-12-19 21:03 PST, Elliott Sprehn
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Elliott Sprehn 2012-12-12 15:39:24 PST
Block level pseudo elements bleed background color to <html>
Comment 1 Elliott Sprehn 2012-12-12 15:55:28 PST
Created attachment 179138 [details]
Patch
Comment 2 WebKit Review Bot 2012-12-12 17:17:39 PST
Comment on attachment 179138 [details]
Patch

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

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Comment 3 Elliott Sprehn 2012-12-12 17:48:50 PST
(In reply to comment #2)
> (From update of attachment 179138 [details])
> Attachment 179138 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/15317012
> 
> New failing tests:
> inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html

This is definitely not related, seeing as this patch only contains tests. :)
Comment 4 Julien Chaffraix 2012-12-13 13:18:04 PST
Comment on attachment 179138 [details]
Patch

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

> LayoutTests/ChangeLog:21
> +        This was reported as a Chromium Bug:
> +        https://code.google.com/p/chromium/issues/detail?id=75544

We usually don't put downstream bug here as they have little signification in WebKit.

> LayoutTests/ChangeLog:23
> +

Extra space.

> LayoutTests/fast/css-generated-content/html-pseudo-background-color.html:8
> +    }

I think you should also check for html:after. Either in this test (with a different color and the condition for passing would be "there should be no background color" or another one.

> LayoutTests/fast/css-generated-content/html-pseudo-background-color.html:11
> +<p>

Your bug should say what it tests (ie include a description). Without the ChangeLog, it is difficult to see what could go wrong.

Including the bug number and title is also nice.
Comment 5 Elliott Sprehn 2012-12-19 20:49:03 PST
(In reply to comment #4)
> (From update of attachment 179138 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179138&action=review
> ...
> 
> Your bug should say what it tests (ie include a description). Without the ChangeLog, it is difficult to see what could go wrong.

So to explain what this bug is about:

RenderObject::isRoot checks if document()->documentElement() == m_node and in the old generated content implementation m_node was set to the generatingNode so for html:before or html:after so isRoot() would return true.

This meant that when we go through RenderBox::paintBackground we would end up painting the generated content's background color over the whole document.

This is fixed by the new generated content implementation that was turned on in r137336 because m_node is now the PseudoElement, not the <html> element.
Comment 6 Elliott Sprehn 2012-12-19 21:03:54 PST
Created attachment 180271 [details]
Patch
Comment 7 WebKit Review Bot 2012-12-20 10:49:49 PST
Comment on attachment 180271 [details]
Patch

Clearing flags on attachment: 180271

Committed r138274: <http://trac.webkit.org/changeset/138274>
Comment 8 WebKit Review Bot 2012-12-20 10:49:56 PST
All reviewed patches have been landed.  Closing bug.