[webkit-reviews] review granted: [Bug 82693] Fix some problems of text field decoration : [Attachment 134752] Patch 3

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Kent Tamura
<tkent at chromium.org>'s request for review:
Bug 82693: Fix some problems of text field decoration
https://bugs.webkit.org/show_bug.cgi?id=82693

Attachment 134752: Patch 3
https://bugs.webkit.org/attachment.cgi?id=134752&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=134752&action=review


> Source/WebCore/GNUmakefile.list.am:2480
> +	Source/WebCore/html/shadow/HTMLShaodwElement.h \

Typo.

> Source/WebCore/html/InputType.cpp:396
>	   root->removeAllChildren();
> +	   root->appendChild(HTMLShadowElement::create(shadowTag,
element()->document()));

Since you are the first consumer of this, I am trying to learn lessons here. It
seems like we need ability to just "disable" a shadow DOM subtree in a tree
stack. This way, you don't need to keep removing and re-adding DOM elements.

> Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:91
> +    getDecorationRootAndDecoratedRoot(input, decorationRoot, existingRoot);

Interesting approach. It's seems we need a way to reference specific shadow DOM
subtrees from the shadowTree(), like getElementById.

> Source/WebCore/html/shadow/TextFieldDecorationElement.h:57
> +    // This function is called just before detaching the decoration. It must
not
> +    // call functions which updating state of the specified HTMLInputElement

> +    // object.

I am still not happy about needing to articulate this in comments. The best
kind of articulation is by design.