Bug 58073 - Presence of shadow DOM should suppress rendering of ordinary child nodes
Summary: Presence of shadow DOM should suppress rendering of ordinary child nodes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 52962
  Show dependency treegraph
 
Reported: 2011-04-07 13:21 PDT by Dominic Cooney
Modified: 2011-04-14 18:06 PDT (History)
3 users (show)

See Also:


Attachments
Should just see keygen select (shadow), but "Hello world" also appears. (33 bytes, text/html)
2011-04-07 13:21 PDT, Dominic Cooney
no flags Details
Patch (5.92 KB, patch)
2011-04-13 14:47 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff
Better patch. (5.92 KB, patch)
2011-04-13 15:11 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dominic Cooney 2011-04-07 13:21:25 PDT
Created attachment 88680 [details]
Should just see keygen select (shadow), but "Hello world" also appears.

When an element has both ordinary children and shadow children, both appear. The presence of shadow children should suppress the rendering of ordinary children.

Repro'd in WK nightly: Version 5.0.4 (6533.20.27, r83161)
Comment 1 Dimitri Glazkov (Google) 2011-04-13 14:27:51 PDT
(In reply to comment #0)
> Created an attachment (id=88680) [details]
> Should just see keygen select (shadow), but "Hello world" also appears.
> 
> When an element has both ordinary children and shadow children, both appear. The presence of shadow children should suppress the rendering of ordinary children.
> 
> Repro'd in WK nightly: Version 5.0.4 (6533.20.27, r83161)

The reduction is wrong (the parser is smart enough to move the text out to be a child of body), but the problem is still present. Fix coming up...
Comment 2 Dimitri Glazkov (Google) 2011-04-13 14:47:32 PDT
Created attachment 89466 [details]
Patch
Comment 3 Ojan Vafai 2011-04-13 15:02:28 PDT
Comment on attachment 89466 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=89466&action=review

> Source/WebCore/dom/Node.cpp:855
> +inline static ContainerNode* shadowRoot(Node* node)

I don't think this inline is really needed. We should reserve inline for cases where we can actually show an improvement, no? But I'm a n00b so feel free to ignore this. :)

> Source/WebCore/dom/Node.cpp:1460
> +inline static bool shouldCreateRendererFor(Node* node, ContainerNode* parentForRenderingAndStyle)

ditto re: inline
Comment 4 Dimitri Glazkov (Google) 2011-04-13 15:11:49 PDT
Created attachment 89475 [details]
Better patch.
Comment 5 WebKit Commit Bot 2011-04-14 18:03:35 PDT
The commit-queue encountered the following flaky tests while processing attachment 89475 [details]:

animations/suspend-resume-animation.html bug 48161 (author: cmarrin@apple.com)
The commit-queue is continuing to process your patch.
Comment 6 WebKit Commit Bot 2011-04-14 18:06:06 PDT
Comment on attachment 89475 [details]
Better patch.

Clearing flags on attachment: 89475

Committed r83922: <http://trac.webkit.org/changeset/83922>
Comment 7 WebKit Commit Bot 2011-04-14 18:06:11 PDT
All reviewed patches have been landed.  Closing bug.