[webkit-reviews] review granted: [Bug 50184] Provide a generic way to store shadowParent on a Node. : [Attachment 75980] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Dec 8 16:33:01 PST 2010
Darin Adler <darin at apple.com> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 50184: Provide a generic way to store shadowParent on a Node.
https://bugs.webkit.org/show_bug.cgi?id=50184
Attachment 75980: Patch
https://bugs.webkit.org/attachment.cgi?id=75980&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=75980&action=review
> WebCore/dom/Node.cpp:492
> + clearFlag(IsShadowRootFlag);
Why clear the flag here? I guess just for clarity or something.
> WebCore/rendering/ShadowElement.h:53
> + virtual void detach()
> + {
> + BaseElement::detach();
> + // FIXME: Remove once shadow DOM uses Element::setShadowRoot().
> + BaseElement::setShadowHost(0);
> + }
Is there a reason to have this in the class definition so it is treated as an
inline request? I know the old code was doing that, but it’s best not to.
> WebCore/rendering/TextControlInnerElements.cpp:106
> + // For elements not in shadow DOM (why?!), add the node to the DOM
normally.
This comment is now confusing to me.
> WebCore/rendering/TextControlInnerElements.h:42
> + virtual void detach();
Can we make it protected or private?
> WebCore/svg/SVGElement.cpp:121
> + ContainerNode* n = parentOrHostNode();
This could be a for loop. Later, I guess.
> WebCore/svg/SVGElement.cpp:136
> + ContainerNode* n = parentOrHostNode();
This could be a for loop. Later, I guess.
More information about the webkit-reviews
mailing list