Bug 64118

Summary: Move all code related to cachedSelection to HTMLTextFormControlElement
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: FormsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, ap, darin, dglazkov, eric, mjs, ojan, sam, tkent
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 47865, 60403    
Attachments:
Description Flags
Removed lots of code ap: review+

Description Ryosuke Niwa 2011-07-07 12:21:58 PDT
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.
Comment 1 Ryosuke Niwa 2011-07-07 12:43:17 PDT
Created attachment 100020 [details]
Removed lots of code
Comment 2 Ryosuke Niwa 2011-07-07 13:41:29 PDT
Ping reviewers because I have a lot of cleanups to do and this patch is blocking my work.
Comment 3 Alexey Proskuryakov 2011-07-07 13:56:40 PDT
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?
Comment 4 Ryosuke Niwa 2011-07-07 14:02:39 PDT
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.
Comment 5 Ryosuke Niwa 2011-07-07 14:03:18 PDT
(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.
Comment 6 Ryosuke Niwa 2011-07-07 14:37:31 PDT
Committed r90591: <http://trac.webkit.org/changeset/90591>