[webkit-reviews] review denied: [Bug 89439] [Forms] Make step action of SpinButtonElement replaceable : [Attachment 148480] Patch 3

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied yosin at chromium.org's request for
review:
Bug 89439: [Forms] Make step action of SpinButtonElement replaceable
https://bugs.webkit.org/show_bug.cgi?id=89439

Attachment 148480: Patch 3
https://bugs.webkit.org/attachment.cgi?id=148480&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=148480&action=review


> Source/WebCore/ChangeLog:29
> +	   (WebCore::TextFieldInputType::stepUpFromRenderer): Moved from
InputType.

I object moving it.  Non-textfield UI might use this feature in the future. 
This feature is not related to "text field" essentially.

> Source/WebCore/html/shadow/TextControlInnerElements.cpp:303
> +		       if (m_stepActionHandler) {
> +			   if (m_upDownState == Up)
> +			       m_stepActionHandler->spinButtonStepUp();
> +			   else
> +			       m_stepActionHandler->spinButtonStepDown();
> +		       }

We had better share the code with the next part.

> Source/WebCore/html/shadow/TextControlInnerElements.cpp:375
> +    if (!m_stepActionHandler)
> +	   return;
> +
> +    if (amount > 0)
> +	   m_stepActionHandler->spinButtonStepUp();
> +    else if (amount < 0)
> +	   m_stepActionHandler->spinButtonStepDown();

We had better share the code with the previous part.

> Source/WebCore/html/shadow/TextControlInnerElements.h:106
> +    static PassRefPtr<SpinButtonElement> create(Document*,
StepActionHandler&);

We should mention that a StepActionHandler implementation must call
removeStepActionHandler().