RESOLVED FIXED 75306
ShadowContentElement query should be able to have fallback element. 
https://bugs.webkit.org/show_bug.cgi?id=75306
Summary ShadowContentElement query should be able to have fallback element.
Shinya Kawanaka
Reported 2011-12-28 05:34:00 PST
In the case issue 75302 is landed, we would like to have fallback element when query does not matches any elements. Currently ShadowContentElement is not allowed to have children, but it can be used for fallback elements.
Attachments
Patch (16.39 KB, patch)
2012-01-12 04:00 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (10.31 KB, patch)
2012-01-16 20:58 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (10.22 KB, patch)
2012-01-16 23:26 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (14.37 KB, patch)
2012-01-18 01:47 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (14.36 KB, patch)
2012-01-18 01:50 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (15.52 KB, patch)
2012-01-18 05:28 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (15.32 KB, patch)
2012-01-18 20:35 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
Dominic Cooney
Comment 1 2012-01-04 18:41:21 PST
When is the right time to move ShadowContentElement out of dom/ and into html/(somewhere) and make it a subtype of HTMLElement instead of Element?
Dimitri Glazkov (Google)
Comment 2 2012-01-04 19:31:18 PST
(In reply to comment #1) > When is the right time to move ShadowContentElement out of dom/ and into html/(somewhere) and make it a subtype of HTMLElement instead of Element? I think we can do this now, right? There's no visible effect at the moment.
Hajime Morrita
Comment 3 2012-01-05 03:00:24 PST
(In reply to comment #2) > (In reply to comment #1) > > When is the right time to move ShadowContentElement out of dom/ and into html/(somewhere) and make it a subtype of HTMLElement instead of Element? > > I think we can do this now, right? There's no visible effect at the moment. No, there is no visible effect. So we can turn this into a subclass of HTMLElement. BTW apparently the comment was jumped from some other bug, it looks.
Shinya Kawanaka
Comment 4 2012-01-12 04:00:50 PST
Created attachment 122208 [details] Patch
Shinya Kawanaka
Comment 5 2012-01-16 20:58:37 PST
Created attachment 122709 [details] Patch
Hajime Morrita
Comment 6 2012-01-16 21:32:07 PST
Comment on attachment 122709 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122709&action=review Could you test some nested cases? > Source/WebCore/ChangeLog:24 > + Changed the order of calculation a bit. Please explain the change concretely. > Source/WebCore/dom/NodeRenderingContext.cpp:86 > + if (!shadowContentElement->inclusions() || !shadowContentElement->inclusions()->first()) { Can inclusions() be null? Is it OK to take the case as a fallback?
Shinya Kawanaka
Comment 7 2012-01-16 23:26:30 PST
(In reply to comment #6) > (From update of attachment 122709 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=122709&action=review > > Could you test some nested cases? > > > Source/WebCore/ChangeLog:24 > > + Changed the order of calculation a bit. > > Please explain the change concretely. Done. > > > Source/WebCore/dom/NodeRenderingContext.cpp:86 > > + if (!shadowContentElement->inclusions() || !shadowContentElement->inclusions()->first()) { > > Can inclusions() be null? Is it OK to take the case as a fallback? Ah, inclusions() won't be null.
Shinya Kawanaka
Comment 8 2012-01-16 23:26:52 PST
Created attachment 122718 [details] Patch
Hajime Morrita
Comment 9 2012-01-17 17:43:50 PST
Comment on attachment 122718 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122718&action=review Basically looks fine. Could you add some more tests? > Source/WebCore/dom/NodeRenderingContext.cpp:85 > + if (!shadowContentElement->inclusions()->first()) { Could you make this conditional a method on ShadowContentElement? hasInclusion() or something? > LayoutTests/fast/dom/shadow/shadow-contents-fallback.html:1 > +<!DOCTYPE html> Oops, I meant to talk about content inside shadow inside content. Such pattern should hit tricky flattening path.
Shinya Kawanaka
Comment 10 2012-01-18 01:47:35 PST
Created attachment 122890 [details] Patch
Shinya Kawanaka
Comment 11 2012-01-18 01:48:07 PST
(In reply to comment #9) > (From update of attachment 122718 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=122718&action=review > > Basically looks fine. Could you add some more tests? > > > Source/WebCore/dom/NodeRenderingContext.cpp:85 > > + if (!shadowContentElement->inclusions()->first()) { > > Could you make this conditional a method on ShadowContentElement? hasInclusion() or something? Done. > > LayoutTests/fast/dom/shadow/shadow-contents-fallback.html:1 > > +<!DOCTYPE html> > > Oops, I meant to talk about content inside shadow inside content. > Such pattern should hit tricky flattening path. I've added a few more tests about it.
Shinya Kawanaka
Comment 12 2012-01-18 01:50:18 PST
Created attachment 122891 [details] Patch
Shinya Kawanaka
Comment 13 2012-01-18 05:28:33 PST
Created attachment 122904 [details] Patch
Hajime Morrita
Comment 14 2012-01-18 18:04:43 PST
Comment on attachment 122904 [details] Patch Thanks for your hard work! Let's land.
Hajime Morrita
Comment 15 2012-01-18 18:05:21 PST
Comment on attachment 122904 [details] Patch Oops. ChangeLog has wrong diff. Could you update it?
Shinya Kawanaka
Comment 16 2012-01-18 20:35:42 PST
Created attachment 123061 [details] Patch
WebKit Review Bot
Comment 17 2012-01-18 23:06:12 PST
Comment on attachment 123061 [details] Patch Clearing flags on attachment: 123061 Committed r105387: <http://trac.webkit.org/changeset/105387>
WebKit Review Bot
Comment 18 2012-01-18 23:06:18 PST
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.