Bug 45903 – Reduce use of HTMLInputElement::inputType so we can remove it later

• Component: Forms
• Status: RESOLVED FIXED

[Darin Adler]

Reduce use of HTMLInputElement::inputType so we can remove it later

[Darin Adler]

Created attachment 67814 [details]
Patch

[Andreas Kling]

Comment on attachment 67814 [details]
Patch

> -                        || (focusedNode->hasTagName(inputTag) && (static_cast<HTMLInputElement*>(focusedNode)->inputType() == HTMLInputElement::TEXT
> -                                                                || static_cast<HTMLInputElement*>(focusedNode)->inputType() == HTMLInputElement::SEARCH)))) {
> +    if (focusedNode && (focusedNode->hasTagName(textareaTag) || (focusedNode->hasTagName(inputTag) && static_cast<HTMLInputElement*>(focusedNode)->isTextField()))) {

Is this behavior change intentional? (isTextField() returns true for COLOR, DATE, DATETIME, DATETIMELOCAL, EMAIL, ISINDEX, MONTH, NUMBER, PASSWORD, SEARCH, TELEPHONE, TEXT, TIME, URL and WEEK.)

[Darin Adler]

(In reply to comment #2)
> Is this behavior change intentional? (isTextField() returns true for COLOR, DATE, DATETIME, DATETIMELOCAL, EMAIL, ISINDEX, MONTH, NUMBER, PASSWORD, SEARCH, TELEPHONE, TEXT, TIME, URL and WEEK.)

Yes.

The code in question relates to changing writing direction when the selection is inside an HTML input element. It says that if the focus is in an input element we should change the writing direction of the element, but if the focus is elsewhere, we should apply the writing direction to the surrounding paragraph. So what matters for the editing operation is whether the input element one that has editable text inside it, or if it is an element that can only be selected as a whole. I believe the right check for that is isTextField.

In the future isTextField will start returning true for those other input types if they are implemented as something other than a text field. And it’s good that the behavior of this function will track such changes.

[Darin Adler]

(In reply to comment #3)
> In the future isTextField will start returning true for those other input types if they are implemented as something other than a text field. And it’s good that the behavior of this function will track such changes.

I meant "returning false".

[Andreas Kling]

Comment on attachment 67814 [details]
Patch

> -        if (!e->hasTagName(inputTag) || static_cast<HTMLInputElement*>(e)->inputType() != HTMLInputElement::IMAGE)
> +        if (!e->hasTagName(inputTag) || static_cast<HTMLInputElement*>(e)->isImageButton())

Second condition is reversed (missing "!")

r=me with that fixed.

[Darin Adler]

(In reply to comment #5)
> (From update of attachment 67814 [details])
> > -        if (!e->hasTagName(inputTag) || static_cast<HTMLInputElement*>(e)->inputType() != HTMLInputElement::IMAGE)
> > +        if (!e->hasTagName(inputTag) || static_cast<HTMLInputElement*>(e)->isImageButton())
> 
> Second condition is reversed (missing "!")

Thanks for catching that!

[Darin Adler]

Committed r67653: <http://trac.webkit.org/changeset/67653>

[WebKit Review Bot]

http://trac.webkit.org/changeset/67653 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Linux ARMv7 Release