Bug 89425 – [Forms] Move isKeyboardFocusable and isMouseFocusable to InputType from HTMLInputElement

• Component: Forms
• Status: RESOLVED FIXED

For introducing time fields UI and code cleanup, input type class should decide of keyboard/mouse focus-able rather than HTMLInputElement.

== Current ==
bool HTMLInputElement::isKeyboardFocusable(KeyboardEvent* event) const
{
    if (isTextField())
        return HTMLTextFormControlElement::isFocusable();
    return HTMLTextFormControlElement::isKeyboardFocusable(event) && m_inputType->isKeyboardFocusable();
}

== Proposed ==

bool HTMLInputElement::isKeyboardFocusable(KeyboardEvent* event) const
{
    return m_inputType->isKeyboardFocusable(event);
}

bool InputType::isKeyboardFocusable(KeyboardEvent* event) const
{
    return element()->isTextFormControlKeyboardFocusable(event); // call new method in HTMLInputElement class
}

bool TextFieldInputType::isKeyboardFocusable(KeyboardEvent* event) const
{
    return element()->isTextFormControlFocusable(); // call new method in HTMLInputElement class
}

Created attachment 148261 [details]
Patch 1

Comment on attachment 148261 [details]
Patch 1

Could you review this patch?
Thanks in advance.

[Kent Tamura]

Comment on attachment 148261 [details]
Patch 1

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

> Source/WebCore/html/HTMLInputElement.h:267
> +    virtual bool isTextFormControlFocusable() const;
> +    virtual bool isTextFormControlKeyboardFocusable(KeyboardEvent*) const;
> +    virtual bool isTextFormControlMouseFocusable() const;

They shouldn't be virtual.
Also, please put them just below setValueInternal() because it is also a helper for *InputType.

> Source/WebCore/html/RadioInputType.cpp:119
> +    if (!element()->isTextFormControlKeyboardFocusable(event))

if (!InputType::isKeyboardFocusable(event))
is better.

Created attachment 148270 [details]
Patch 2

Comment on attachment 148270 [details]
Patch 2

Could you review this patch?
Thanks in advance.

= Changes since last review ==
* Remove "virtual" from isTextFormControl*Focusable method declarations
** as suggested
* Call InputType::isKeyboardFocusable() in RadioInputType::isKeyboardFocusable
** as suggested

[Build Bot]

Comment on attachment 148270 [details]
Patch 2

Attachment 148270 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12972717

Created attachment 148274 [details]
Patch 3

Comment on attachment 148274 [details]
Patch 3

Could you review this patch?
Thanks in advance.

= Changes since last review ==
* Remove "virtual" from isTextFormControl*Focusable method declarations
** as suggested
* Call InputType::isKeyboardFocusable() in RadioInputType::isKeyboardFocusable
** as suggested
* Remove unused prameter in TextFieldInputType::isKeyboardFocusable
** Failed on Mac build

[Kent Tamura]

Comment on attachment 148274 [details]
Patch 3

ok

[WebKit Review Bot]

Comment on attachment 148274 [details]
Patch 3

Clearing flags on attachment: 148274

Committed r120695: <http://trac.webkit.org/changeset/120695>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.