Bug 90079

Summary: [CSS Regions] It should be possible to specify ::BEFORE/AFTER as regions
Product: WebKit Reporter: Andrei Bucur <abucur>
Component: WebCore Misc.Assignee: Andrei Bucur <abucur>
Status: RESOLVED FIXED    
Severity: Normal CC: donggwan.kim, eric, esprehn, hyatt, stearns, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 57312    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Andrei Bucur
Reported 2012-06-27 08:56:06 PDT
Currently, the implementation assumes the RenderRegions have a direct node attached to them. This is not true in the case of pseudo-elements renderers.
Attachments
Patch (15.94 KB, patch)
2012-06-28 07:10 PDT, Andrei Bucur 		no flags
DetailsFormatted DiffDiff
Patch (12.95 KB, patch)
2012-08-30 02:20 PDT, Andrei Bucur 		no flags
DetailsFormatted DiffDiff
Patch (18.63 KB, patch)
2012-08-31 06:01 PDT, Andrei Bucur 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Andrei Bucur
Comment 1 2012-06-28 07:10:02 PDT
Created attachment 149947 [details] Patch
Andrei Bucur
Comment 2 2012-08-30 02:20:33 PDT
Created attachment 161429 [details] Patch
Dave Hyatt
Comment 3 2012-08-30 10:50:34 PDT
Comment on attachment 161429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161429&action=review Just one question about this one. > Source/WebCore/rendering/RenderRegion.h:50 > + // Regions can't have any children. > + virtual bool isChildAllowed(RenderObject*, RenderStyle*) const { return false; } This is weird to me. Eventually we want to turn RenderRegions into RenderBlocks so that they can have ::before and ::after children. In other words, we do want to support children on RenderRegions eventually, so why do we turn that off?
Andrei Bucur
Comment 4 2012-08-30 11:27:32 PDT
(In reply to comment #3) > (From update of attachment 161429 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=161429&action=review > > Just one question about this one. > > > Source/WebCore/rendering/RenderRegion.h:50 > > + // Regions can't have any children. > > + virtual bool isChildAllowed(RenderObject*, RenderStyle*) const { return false; } > > This is weird to me. Eventually we want to turn RenderRegions into RenderBlocks so that they can have ::before and ::after children. In other words, we do want to support children on RenderRegions eventually, so why do we turn that off? That comment was supposed to be read more like "FIXME PLEASE: Regions can't have any children because they are RenderReplaced and every time in RenderObjectChildList someone tries to add a child on generatedContentContainer an ASSERT will trigger... (make me RenderBlock!)"
Dave Hyatt
Comment 5 2012-08-30 11:29:44 PDT
Ah ok, got it. Go ahead and put in a FIXME then and put up a new patch and I will r+ and cq+.
Andrei Bucur
Comment 6 2012-08-30 11:31:53 PDT
(In reply to comment #5) > Ah ok, got it. Go ahead and put in a FIXME then and put up a new patch and I will r+ and cq+. Tomorrow, bed time now.
Andrei Bucur
Comment 7 2012-08-31 06:01:49 PDT
Created attachment 161683 [details] Patch
Andrei Bucur
Comment 8 2012-08-31 06:07:58 PDT
Comment on attachment 161683 [details] Patch The patch is a little different. I've realised it's pointless to override isChildAllowed() now and I think this is a better way to do it. Basically if the pseudo-element is a region, the content values are ignored. I think this is inline with the expected behaviour as well. A pseudo-element with flow-from but no content values should still be displayed.
Dave Hyatt
Comment 9 2012-08-31 08:44:53 PDT
Comment on attachment 161683 [details] Patch r=me
WebKit Review Bot
Comment 10 2012-08-31 08:49:45 PDT
Comment on attachment 161683 [details] Patch Clearing flags on attachment: 161683 Committed r127269: <http://trac.webkit.org/changeset/127269>
WebKit Review Bot
Comment 11 2012-08-31 08:49:48 PDT
All reviewed patches have been landed. Closing bug.
Andrei Bucur
Comment 12 2012-09-18 08:20:24 PDT
*** Bug 92125 has been marked as a duplicate of this bug. ***
Mihai Balan
Comment 13 2012-10-01 01:12:42 PDT
*** Bug 75587 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.