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.
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?
(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.
(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.
Created attachment 122208 [details] Patch
Created attachment 122709 [details] Patch
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?
(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.
Created attachment 122718 [details] Patch
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.
Created attachment 122890 [details] Patch
(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.
Created attachment 122891 [details] Patch
Created attachment 122904 [details] Patch
Comment on attachment 122904 [details] Patch Thanks for your hard work! Let's land.
Comment on attachment 122904 [details] Patch Oops. ChangeLog has wrong diff. Could you update it?
Created attachment 123061 [details] Patch
Comment on attachment 123061 [details] Patch Clearing flags on attachment: 123061 Committed r105387: <http://trac.webkit.org/changeset/105387>
All reviewed patches have been landed. Closing bug.