Bug 67320 – Move text() and textWithHardLineBreaks() from RenderTextControl to HTMLTextFormControlElement

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Ryosuke Niwa]

text() and textWithHardLineBreaks() don't depend anything in RenderText, and should be moved to HTMLTextFormControlElement.

[Ryosuke Niwa]

Created attachment 105838 [details]
refactoring

[Ryosuke Niwa]

Comment on attachment 105838 [details]
refactoring

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

> Source/WebCore/html/HTMLTextAreaElement.cpp:-175
> -    // FIXME: It's not acceptable to ignore the HardWrap setting when there is no renderer.
> -    // While we have no evidence this has ever been a practical problem, it would be best to fix it some day.

This comment is moved into valueWithHardLineBreaks.

> Source/WebCore/html/HTMLTextFormControlElement.cpp:524
> +        return value();

We call value() instead of emptyString() here (and 2 other places below).  These are only differences between HTMLTextFormControlElement::valueWithHardLineBreaks and RenderTextControl::textWithHardLineBreaks

> Source/WebCore/html/NumberInputType.cpp:321
> +    return element()->renderer() && !isAcceptableValue(element()->innerTextValue());

I bet these input type functions want to call value() instead of innerTextValue() but that can be done in a separate patch.

[Gyuyoung Kim]

Comment on attachment 105838 [details]
refactoring

Attachment 105838 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9563999

[Early Warning System Bot]

Comment on attachment 105838 [details]
refactoring

Attachment 105838 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9578085

[Ryosuke Niwa]

Created attachment 105848 [details]
fixed efl build

[Ryosuke Niwa]

Any reviewers?  This patch is blocking my work for the bug 66241.

[Darin Adler]

Comment on attachment 105848 [details]
fixed efl build

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

r=me

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1956
> +        return toRenderTextControl(m_renderer)->textFormControlElement()->value();

Can textFormControlElement() ever be 0?

[Ryosuke Niwa]

Comment on attachment 105848 [details]
fixed efl build

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

>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1956
>> +        return toRenderTextControl(m_renderer)->textFormControlElement()->value();
> 
> Can textFormControlElement() ever be 0?

Only if RenderObject::m_node is 0.  I guess I should check that.  Or that I just need to move this line below the nullity check below.

[Ryosuke Niwa]

Thanks for the review!

[Early Warning System Bot]

Comment on attachment 105848 [details]
fixed efl build

Attachment 105848 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9572727

[Ryosuke Niwa]

Committed r94252: <http://trac.webkit.org/changeset/94252>

[Kent Tamura]

Comment on attachment 105838 [details]
refactoring

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

>> Source/WebCore/html/NumberInputType.cpp:321
>> +    return element()->renderer() && !isAcceptableValue(element()->innerTextValue());
> 
> I bet these input type functions want to call value() instead of innerTextValue() but that can be done in a separate patch.

It should be innerTextValue().
LayoutTests/fast/forms/input-number-unacceptable-style.html tests it.