Bug 114268

Summary: [CSS Regions] Regions don't create a stacking context for their contents
Product: WebKit Reporter: Mihai Balan <mibalan>
Component: CSSAssignee: Max Vujovic <mvujovic>
Status: RESOLVED FIXED    
Severity: Normal CC: achicu, commit-queue, esprehn+autocc, glenn, WebkitBugTracker
Priority: P2 Keywords: AdobeTracked
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 57312    
Attachments:
Description Flags
W3C test showcasing the problem
none
Patch
achicu: review-, mvujovic: commit-queue-
Patch none

Mihai Balan
Reported 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)
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-
DetailsFormatted DiffDiff
Patch (5.75 KB, patch)
2013-06-11 13:34 PDT, Max Vujovic 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Max Vujovic
Comment 1 2013-06-11 10:54:19 PDT
Created attachment 204343 [details] Patch
Alexandru Chiculita
Comment 2 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.
Max Vujovic
Comment 3 2013-06-11 13:34:15 PDT
Created attachment 204356 [details] Patch Updated patch based on Alex's comments.
Max Vujovic
Comment 4 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.
Alexandru Chiculita
Comment 5 2013-06-11 14:33:00 PDT
Comment on attachment 204356 [details] Patch r=me
Max Vujovic
Comment 6 2013-06-11 14:33:54 PDT
Comment on attachment 204356 [details] Patch Thanks for the review! Setting cq+.
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2013-06-11 14:55:19 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.