To provide flattened-tree traversal, we need to traverse from forwarded nodes to hosting content element. But we currently maintain only references from content element to forwarded nodes.
Created attachment 100666 [details] Patch
Hi Dimitri, could you take a look? This patch introduces the included node to content element hash. -- I feel relationship between ShadowContentElement, ShadowInclusion, ShadowContentSelector becomes complicated, so I'll do some reorg after this patch. My plan is to - move ShadowContentSelector to a member variable of ShadowRoot, - move ShadowContentSet to ShadowContentSelector Then we can hide content-forwarding mechanism behind ShadowContentSelector.
Comment on attachment 100666 [details] Patch Attachment 100666 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9016532 New failing tests: fast/html/details-nested-2.html
Created attachment 100668 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 100666 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100666&action=review I am ok with the approach as an iteration, but the bots are angry. > Source/WebCore/dom/ShadowRoot.cpp:33 > +#include "ShadowContentElement.h" // XXX: should be ShadowInclusion.h or something Probably don't want that comment left in :)
Created attachment 100750 [details] Remove wrong comment, fixed build and test breakage
Hi Dimitri, thanks for reviewing this! > > Source/WebCore/dom/ShadowRoot.cpp:33 > > +#include "ShadowContentElement.h" // XXX: should be ShadowInclusion.h or something > > Probably don't want that comment left in :) Oops... thanks for the catch. Removed. I hope triple-x makes style bot uncomfortable.
Comment on attachment 100750 [details] Remove wrong comment, fixed build and test breakage View in context: https://bugs.webkit.org/attachment.cgi?id=100750&action=review > Source/WebKit2/win/WebKit2.def:140 > + ??0NodeRenderingContext@WebCore@@QAE@PAVNode@1@@Z > + ??1NodeRenderingContext@WebCore@@QAE@XZ > + ?toNode@WebCore@@YAPAVNode@1@VJSValue@JSC@@@Z Is this indent correct?
(In reply to comment #8) > (From update of attachment 100750 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=100750&action=review > > > Source/WebKit2/win/WebKit2.def:140 > > + ??0NodeRenderingContext@WebCore@@QAE@PAVNode@1@@Z > > + ??1NodeRenderingContext@WebCore@@QAE@XZ > > + ?toNode@WebCore@@YAPAVNode@1@VJSValue@JSC@@@Z > > Is this indent correct? Ouch, I'll untabify them before landing. Thanks for take a look again, Dimitri!
Created attachment 100758 [details] For CQ
Comment on attachment 100758 [details] For CQ Clearing flags on attachment: 100758 Committed r90976: <http://trac.webkit.org/changeset/90976>
All reviewed patches have been landed. Closing bug.
Committed r90987: <http://trac.webkit.org/changeset/90987>
Rolled out at http://trac.webkit.org/changeset/90987 Due to gtk build error http://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release/builds/15574/steps/compile-webkit/logs/stdio
Created attachment 100944 [details] For asking EWS
I finally had time to look at this. Adding _ZN7WebCore6toNodeEN3JSC7JSValueE; in Source/autotools/symbols.filter should fix this build issue. So, please re-land the patch with that little update^^ :) No clean build should be needed. Thanks to mrobinson for the help on this.
(In reply to comment #16) > I finally had time to look at this. Adding _ZN7WebCore6toNodeEN3JSC7JSValueE; in Source/autotools/symbols.filter should fix this build issue. > > So, please re-land the patch with that little update^^ :) > > No clean build should be needed. > Thanks to mrobinson for the help on this. I didn't see comment 15. So yeah, this new patch should build indeed on GTK.
> > No clean build should be needed. > > Thanks to mrobinson for the help on this. > > I didn't see comment 15. So yeah, this new patch should build indeed on GTK. Hi Philippe, Thanks so much for your investigation! That's really helpful because I don't have a way to do GTK build. I'll retry this patch now.
Comment on attachment 100944 [details] For asking EWS Clearing flags on attachment: 100944 Committed r91235: <http://trac.webkit.org/changeset/91235>