Bug 106533 – [Refactoring] HTMLTextFormControlElement should use shadowHost instead of shadowAncestorNode

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Shinya Kawanaka]

shadowAncestorNode() is deprecated. We would like to use shadowHost.

[Shinya Kawanaka]

Created attachment 182076 [details]
Patch

[Kent Tamura]

Comment on attachment 182076 [details]
Patch

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

ok

> Source/WebCore/html/HTMLTextFormControlElement.cpp:647
> +    return ancestor ? toTextFormControl(ancestor) : 0;

Not your fault, but this code looks dangerous.  Who ensures that the position points shadow nodes in text input or textarea?

[Shinya Kawanaka]

I'll land this patch anyway, and I'll file another bug to address the issue.

[Shinya Kawanaka]

https://bugs.webkit.org/show_bug.cgi?id=107089

[WebKit Review Bot]

Comment on attachment 182076 [details]
Patch

Clearing flags on attachment: 182076

Committed r139962: <http://trac.webkit.org/changeset/139962>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 182076 [details]
Patch

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

> Source/WebCore/html/HTMLTextFormControlElement.cpp:646
> +    Node* ancestor = container->shadowHost();

The ancestor variable should be of type Element*, not Node*.

>> Source/WebCore/html/HTMLTextFormControlElement.cpp:647
>> +    return ancestor ? toTextFormControl(ancestor) : 0;
> 
> Not your fault, but this code looks dangerous.  Who ensures that the position points shadow nodes in text input or textarea?

Good point, this can be a bad cast! We need a type check.

[Shinya Kawanaka]

Comment on attachment 182076 [details]
Patch

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

>>> Source/WebCore/html/HTMLTextFormControlElement.cpp:647
>>> +    return ancestor ? toTextFormControl(ancestor) : 0;
>> 
>> Not your fault, but this code looks dangerous.  Who ensures that the position points shadow nodes in text input or textarea?
> 
> Good point, this can be a bad cast! We need a type check.

toTextFormControl() returns 0 if it's not TextFormControl.
So this would be safe, but I think it does not align our recent code convention. Maybe refactoring is desired.