Bug 90079 – [CSS Regions] It should be possible to specify ::BEFORE/AFTER as regions

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Andrei Bucur]

Currently, the implementation assumes the RenderRegions have a direct node attached to them. This is not true in the case of  pseudo-elements renderers.

[Andrei Bucur]

Created attachment 149947 [details]
Patch

[Andrei Bucur]

Created attachment 161429 [details]
Patch

[Dave Hyatt]

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]

(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]

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]

(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]

Created attachment 161683 [details]
Patch

[Andrei Bucur]

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 on attachment 161683 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 161683 [details]
Patch

Clearing flags on attachment: 161683

Committed r127269: <http://trac.webkit.org/changeset/127269>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Andrei Bucur]

*** Bug 92125 has been marked as a duplicate of this bug. ***

[Mihai Balan]

*** Bug 75587 has been marked as a duplicate of this bug. ***