Bug 114268 - [CSS Regions] Regions don't create a stacking context for their contents
Summary: [CSS Regions] Regions don't create a stacking context for their contents
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Max Vujovic
URL:
Keywords: AdobeTracked
Depends on:
Blocks: 57312
  Show dependency treegraph
 
Reported: 2013-04-09 05:00 PDT by Mihai Balan
Modified: 2013-06-11 14:55 PDT (History)
5 users (show)

See Also:


Attachments
W3C test showcasing the problem (1.73 KB, text/html)
2013-04-09 05:00 PDT, Mihai Balan
no flags Details
Patch (5.73 KB, patch)
2013-06-11 10:54 PDT, Max Vujovic
achicu: review-
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (5.75 KB, patch)
2013-06-11 13:34 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Balan 2013-04-09 05:00:43 PDT
Created attachment 197030 [details]
W3C test showcasing the problem

The CSS Regions spec states that regions should create a stacking context for their contents.
However, it currently doesn't look like this is happening

Steps to reproduce:
Open attached file in browser

EXPECTED
The purple square is drawn on top of the other squares

ACTUAL
The purple square is drawn under the yellow square (actually under all the other three squares)
Comment 1 Max Vujovic 2013-06-11 10:54:19 PDT
Created attachment 204343 [details]
Patch
Comment 2 Alexandru Chiculita 2013-06-11 11:41:04 PDT
Comment on attachment 204343 [details]
Patch

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

> Source/WebCore/rendering/RenderRegion.cpp:166
> +    bool isPaintingBackground = (paintInfo.phase == PaintPhaseBlockBackground || paintInfo.phase == PaintPhaseChildBlockBackground);
> +    if (!isValid() || (!isPaintingBackground && paintInfo.phase != PaintPhaseSelection))

Let's extract this into something like "bool shouldPaint". It looks bad with so many negations in it.
Comment 3 Max Vujovic 2013-06-11 13:34:15 PDT
Created attachment 204356 [details]
Patch

Updated patch based on Alex's comments.
Comment 4 Max Vujovic 2013-06-11 13:37:05 PDT
(In reply to comment #2)
> (From update of attachment 204343 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204343&action=review
> 
> > Source/WebCore/rendering/RenderRegion.cpp:166
> > +    bool isPaintingBackground = (paintInfo.phase == PaintPhaseBlockBackground || paintInfo.phase == PaintPhaseChildBlockBackground);
> > +    if (!isValid() || (!isPaintingBackground && paintInfo.phase != PaintPhaseSelection))
> 
> Let's extract this into something like "bool shouldPaint". It looks bad with so many negations in it.

Good suggestion. I pulled the isValid() into it's own if statement. Now this if statement only contains the phase checking. I ended up not needing a boolean.
Comment 5 Alexandru Chiculita 2013-06-11 14:33:00 PDT
Comment on attachment 204356 [details]
Patch

r=me
Comment 6 Max Vujovic 2013-06-11 14:33:54 PDT
Comment on attachment 204356 [details]
Patch

Thanks for the review!

Setting cq+.
Comment 7 WebKit Commit Bot 2013-06-11 14:55:17 PDT
Comment on attachment 204356 [details]
Patch

Clearing flags on attachment: 204356

Committed r151475: <http://trac.webkit.org/changeset/151475>
Comment 8 WebKit Commit Bot 2013-06-11 14:55:19 PDT
All reviewed patches have been landed.  Closing bug.