WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
90079
[CSS Regions] It should be possible to specify ::BEFORE/AFTER as regions
https://bugs.webkit.org/show_bug.cgi?id=90079
Summary
[CSS Regions] It should be possible to specify ::BEFORE/AFTER as regions
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
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
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.
Top of Page
Format For Printing
XML
Clone This Bug