Bug 93928 - [Forms] Move wheel event handler to spin button class
Summary: [Forms] Move wheel event handler to spin button 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: 94166
  Show dependency treegraph
 
Reported: 2012-08-13 21:10 PDT by yosin
Modified: 2012-08-15 18:37 PDT (History)
3 users (show)

See Also:


Attachments
Patch 1 (11.14 KB, patch)
2012-08-13 23:19 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch 2 (11.18 KB, patch)
2012-08-14 00:53 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-08-13 21:10:42 PDT
In current implementation, wheel event handler is implemented in TextFieldInputType and called from NumberInputType and BaseDateTimeInputType class.

To shared wheel event handler code within them and multiple field time UI, I would like to move implementation of wheel event handler into SpinButton class.
Comment 1 yosin 2012-08-13 23:19:58 PDT
Created attachment 158230 [details]
Patch 1
Comment 2 yosin 2012-08-13 23:21:03 PDT
Comment on attachment 158230 [details]
Patch 1

Could you review this patch
Thanks in advance.
Comment 3 Kent Tamura 2012-08-14 00:08:03 PDT
Comment on attachment 158230 [details]
Patch 1

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

> Source/WebCore/html/shadow/TextControlInnerElements.cpp:354
> +    HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowHost());
> +    if (input->disabled() || input->readOnly() || !input->focused())
> +        return;
> +

You wanted to remove HTMLInputElement dependency from SpinButtonElement, right?
Comment 4 yosin 2012-08-14 00:48:32 PDT
(In reply to comment #3)
> (From update of attachment 158230 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=158230&action=review
> 
> > Source/WebCore/html/shadow/TextControlInnerElements.cpp:354
> > +    HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowHost());
> > +    if (input->disabled() || input->readOnly() || !input->focused())
> > +        return;
> > +
> 
> You wanted to remove HTMLInputElement dependency from SpinButtonElement, right?

Yes, I really want to do so. 
I would like to make this patch to be handling wheel event only.

I'll free SpinButtonElement ffrom HTMLInputElement dependency in other patches like:

Patch 1. Do svn copy TextControllInnerElements.{cpp,h} to SpinButtonElement.{cpp,h}
Patch 2. Rename SpinButtonActionHandler to SpinButtonElementOwner with willHandleStepAction()
Comment 5 yosin 2012-08-14 00:53:15 PDT
Created attachment 158249 [details]
Patch 2
Comment 6 yosin 2012-08-14 00:54:23 PDT
Comment on attachment 158249 [details]
Patch 2

Could you review this patch?
Thanks in advance.

= Changes since the last review =
* (SpinButtonElement::forwardEvent): Check SpinButtonElement rendered(== calling renderBox() != nullptr)
Comment 7 Kent Tamura 2012-08-14 01:27:40 PDT
Comment on attachment 158249 [details]
Patch 2

ok
Comment 8 yosin 2012-08-14 01:44:55 PDT
Comment on attachment 158249 [details]
Patch 2

Clearing flags on attachment: 158249

Committed r125522: <http://trac.webkit.org/changeset/125522>
Comment 9 yosin 2012-08-14 01:44:59 PDT
All reviewed patches have been landed.  Closing bug.