Bug 64118 - Move all code related to cachedSelection to HTMLTextFormControlElement
Summary: Move all code related to cachedSelection to HTMLTextFormControlElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 47865 60403
  Show dependency tree / graph
 
Reported: 2011-07-07 12:21 PDT by Ryosuke Niwa
Modified: 2011-07-07 14:37 PDT (History)
9 users (show)

See Also:

Attachments
Removed lots of code (17.90 KB, patch)
2011-07-07 12:43 PDT, Ryosuke Niwa 		ap: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>