Bug 90079 - [CSS Regions] It should be possible to specify ::BEFORE/AFTER as regions
Summary: [CSS Regions] It should be possible to specify ::BEFORE/AFTER as regions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andrei Bucur
URL:
Keywords:
: 75587 92125 (view as bug list)
Depends on:
Blocks: 57312
  Show dependency treegraph
 
Reported: 2012-06-27 08:56 PDT by Andrei Bucur
Modified: 2012-10-01 01:12 PDT (History)
6 users (show)

See Also:


Attachments
Patch (15.94 KB, patch)
2012-06-28 07:10 PDT, Andrei Bucur
no flags Details | Formatted Diff | Diff
Patch (12.95 KB, patch)
2012-08-30 02:20 PDT, Andrei Bucur
no flags Details | Formatted Diff | Diff
Patch (18.63 KB, patch)
2012-08-31 06:01 PDT, Andrei Bucur
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Bucur 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.
Comment 1 Andrei Bucur 2012-06-28 07:10:02 PDT
Created attachment 149947 [details]
Patch
Comment 2 Andrei Bucur 2012-08-30 02:20:33 PDT
Created attachment 161429 [details]
Patch
Comment 3 Dave Hyatt 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?
Comment 4 Andrei Bucur 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!)"
Comment 5 Dave Hyatt 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+.
Comment 6 Andrei Bucur 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.
Comment 7 Andrei Bucur 2012-08-31 06:01:49 PDT
Created attachment 161683 [details]
Patch
Comment 8 Andrei Bucur 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.
Comment 9 Dave Hyatt 2012-08-31 08:44:53 PDT
Comment on attachment 161683 [details]
Patch

r=me
Comment 10 WebKit Review Bot 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>
Comment 11 WebKit Review Bot 2012-08-31 08:49:48 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Andrei Bucur 2012-09-18 08:20:24 PDT
*** Bug 92125 has been marked as a duplicate of this bug. ***
Comment 13 Mihai Balan 2012-10-01 01:12:42 PDT
*** Bug 75587 has been marked as a duplicate of this bug. ***