| Summary: | [ShadowContentElement] forwarded node should be able to access its hosting content element. | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Hajime Morrita <morrita> | ||||||||||||
| Component: | DOM | Assignee: | Hajime Morrita <morrita> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | dglazkov, pnormand, webkit.review.bot | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Bug Depends on: | 64252, 64517 | ||||||||||||||
| Bug Blocks: | 64072 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Hajime Morrita
2011-07-10 23:30:21 PDT
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> All reviewed patches have been landed. Closing bug. |