Bug 92230 - [Forms] Move HTMLInputElement::updateInnerTextValue to InputType class
Summary: [Forms] Move HTMLInputElement::updateInnerTextValue to InputType class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on:
Blocks: 88970
  Show dependency treegraph
 
Reported: 2012-07-25 01:34 PDT by yosin
Modified: 2012-07-25 18:01 PDT (History)
3 users (show)

See Also:


Attachments
Patch 1 (4.47 KB, patch)
2012-07-25 02:02 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch 1 (9.11 KB, patch)
2012-07-25 03:04 PDT, yosin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2012-07-25 01:34:39 PDT
To implement, multiple field time input UI, we need to tell TimeInputType about changing value. This can be done by making HTMLInputElement::updateInnerTextValue() to call InputType::updateInnerValue().

Here is list of call site of updateInnerTextValue()
 - DateInputValue::handleBlurEvent()
 - HTMLInputElement::copyNonAttributePropertiesFromElement()
 - HTMLInputElement::parseAttribute() -- for changing value via setAttribute()
 - HTMLInputElement::setSuggestedValue()
 - HTMLInputElement::updateType()
 - NumberInputType::handleBlurEvent()
 - TextFieldInputType::setValue()
Comment 1 yosin 2012-07-25 02:02:11 PDT
Created attachment 154295 [details]
Patch 1
Comment 2 yosin 2012-07-25 02:15:08 PDT
Comment on attachment 154295 [details]
Patch 1

Could you review this patch?
Thanks in advance.
Comment 3 Kent Tamura 2012-07-25 02:31:27 PDT
Comment on attachment 154295 [details]
Patch 1

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

> Source/WebCore/html/HTMLInputElement.cpp:493
>  void HTMLInputElement::updateInnerTextValue()
>  {

Please remove HTMLInputElement::updateInnerTextValue().

> Source/WebCore/html/InputType.h:262
> +    virtual void updateInnerValue() { }

Do not define function body in a header for virtual functions.  It wastes build time.

> Source/WebCore/html/TextFieldInputType.cpp:453
> +void TextFieldInputType::updateInnerValue()

The function name should be updateInnerTextValue() because of consistency with 'innerText' functions though I don't like this name.
Comment 4 yosin 2012-07-25 03:04:28 PDT
Created attachment 154309 [details]
Patch 1
Comment 5 yosin 2012-07-25 03:06:11 PDT
Comment on attachment 154309 [details]
Patch 1

Could you review this patch?
Thanks in advance.

= Changes since the last review =
* Remove HTMLInputElement::updateInnerTextValue()
* Update call sites of HTMLInputElement::updateInnerTextValue()
* Use InputType::updateInnerTextValue() instead of updateInnerValue()
Comment 6 Kent Tamura 2012-07-25 04:12:26 PDT
Comment on attachment 154309 [details]
Patch 1

ok
Comment 7 yosin 2012-07-25 18:01:21 PDT
Comment on attachment 154309 [details]
Patch 1

Clearing flags on attachment: 154309

Committed r123687: <http://trac.webkit.org/changeset/123687>
Comment 8 yosin 2012-07-25 18:01:26 PDT
All reviewed patches have been landed.  Closing bug.