Bug 64118 – Move all code related to cachedSelection to HTMLTextFormControlElement

• Component: Forms
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Right now, HTMLFormControlElement, HTMLInputElement, HTMLTextAreaElement, RenderTextControl, RenderTextControlSingleLine, and RenderTextControlMultipleLine are all aware of cachedSelection.  But this simply is an artifact of WML, and we should be able to push all methods and variables related to cachedSelection into HTMLFormControlElement.

[Ryosuke Niwa]

Created attachment 100020 [details]
Removed lots of code

[Ryosuke Niwa]

Ping reviewers because I have a lot of cleanups to do and this patch is blocking my work.

[Alexey Proskuryakov]

Comment on attachment 100020 [details]
Removed lots of code

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

> Source/WebCore/html/HTMLFormControlElement.cpp:715
> +    if (!renderer() || !isTextFormControl())

When is HTMLTextFormControlElement not a text form control?

> Source/WebCore/html/HTMLFormControlElement.h:238
> +    int m_cachedSelectionStart;
> +    int m_cachedSelectionEnd;

What prevents these from being private?

[Ryosuke Niwa]

Comment on attachment 100020 [details]
Removed lots of code

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

Thanks for the review!

>> Source/WebCore/html/HTMLFormControlElement.cpp:715
>> +    if (!renderer() || !isTextFormControl())
> 
> When is HTMLTextFormControlElement not a text form control?

input[type=checkbox], input[type=radio], etc...

>> Source/WebCore/html/HTMLFormControlElement.h:238
>> +    int m_cachedSelectionEnd;
> 
> What prevents these from being private?

Oh, right.  Will make them private.  I had to make them protected when HTMLInputElement and HTMLTextAreaElement still access those variables directly.

[Ryosuke Niwa]

(In reply to comment #4)
> >> Source/WebCore/html/HTMLFormControlElement.cpp:715
> >> +    if (!renderer() || !isTextFormControl())
> > 
> > When is HTMLTextFormControlElement not a text form control?
> 
> input[type=checkbox], input[type=radio], etc...

I agree that it's very confusing and we should probably rename it to isTextField.

[Ryosuke Niwa]

Committed r90591: <http://trac.webkit.org/changeset/90591>