Bug 67175 – Get rid of toInputElement()

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Forms

[Ryosuke Niwa]

Yet another left over from WML :(

[Ryosuke Niwa]

Created attachment 105656 [details]
work in progress

It's clear to me that we need some sort of a helper function.

[Early Warning System Bot]

Comment on attachment 105656 [details]
work in progress

Attachment 105656 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9566459

[Gyuyoung Kim]

Comment on attachment 105656 [details]
work in progress

Attachment 105656 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9564521

[WebKit Review Bot]

Comment on attachment 105656 [details]
work in progress

Attachment 105656 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9567495

[Ryosuke Niwa]

Created attachment 105677 [details]
work in progress 2

[Ryosuke Niwa]

Created attachment 105698 [details]
Patch

[Darin Adler]

Comment on attachment 105698 [details]
Patch

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

> Source/WebCore/html/shadow/TextControlInnerElements.cpp:137
> +    if (toHTMLInputElement(host)->maxResults() < 0)
> +        return decorationId;
> +    if (toHTMLInputElement(host)->maxResults() > 0)
> +        return resultsId;

Seems a little weak to call maxResults twice here. But maybe it’s just an inlined accessor and so no additional performance cost.

[Ryosuke Niwa]

(In reply to comment #7)
> (From update of attachment 105698 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105698&action=review
> 
> > Source/WebCore/html/shadow/TextControlInnerElements.cpp:137
> > +    if (toHTMLInputElement(host)->maxResults() < 0)
> > +        return decorationId;
> > +    if (toHTMLInputElement(host)->maxResults() > 0)
> > +        return resultsId;
> 
> Seems a little weak to call maxResults twice here. But maybe it’s just an inlined accessor and so no additional performance cost.

Huh, I didn't realize that.  Will fix before landing it.

[Ryosuke Niwa]

Comment on attachment 105698 [details]
Patch

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

> Source/WebCore/html/HTMLInputElement.h:362
> +    return static_cast<const HTMLInputElement*>(node);

Oops, I somehow have const here :(  Will fix also.

[WebKit Review Bot]

Comment on attachment 105698 [details]
Patch

Attachment 105698 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9565603

[Ryosuke Niwa]

Committed r94142: <http://trac.webkit.org/changeset/94142>

[Ryosuke Niwa]

HTMLIsIndexElement inherits from HTMLInputElement.  Given that, I'm not sure if this refactoring is a good idea. For one, hasTagName is much slower, but also explicitly checking for the tag name might be more error-prone.

[Ryosuke Niwa]

The patch was reverted in http://trac.webkit.org/changeset/94149.

[Collabora GTK+ EWS bot]

Comment on attachment 105698 [details]
Patch

Attachment 105698 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9566719

[Darin Adler]

Sorry for leading you astray, Ryosuke. I still don't like the virtual function so much, but we can probably live with this.

[WebKit Review Bot]

Comment on attachment 105698 [details]
Patch

Attachment 105698 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9580034

[WebKit Review Bot]

Comment on attachment 105698 [details]
Patch

Attachment 105698 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9578194