Bug 48177 – Expose sendChangeEvent parameter in WebInputElement::setValue() API

• Component: New Bugs
• Status: RESOLVED FIXED

[Ilya Sherman]

The Chromium API is missing the second parameter for WebInputElement::setValue(), which is a boolean value controlling whether or not to send an onChange() JS event.  We want to be able to send the onChange() event for form autofill; but this is not possible with the current API.

[Ilya Sherman]

Created attachment 71630 [details]
Patch

[Ilya Sherman]

My thinking is to have both versions of setValue() temporarily.  Once Chromium code is updated (which I will do once this patch lands), I'll post a new WebKit patch to remove the single-parameter version of setValue().

[WebKit Review Bot]

Attachment 71630 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/chromium/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ilya Sherman]

Created attachment 71631 [details]
Patch

[Kent Tamura]

Comment on attachment 71631 [details]
Patch

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

> WebKit/chromium/public/WebInputElement.h:69
> +        WEBKIT_API void setValue(const WebString&); // TODO(isherman): Remove once chromium code is updated.

We use some default arguments in WebKit API.

WebKit/chromium/public/WebCursorInfo.h:    explicit WebCursorInfo(Type type = TypePointer)
WebKit/chromium/public/WebFrame.h:                          bool replace = false) = 0;
WebKit/chromium/public/WebFrame.h:                                bool replace = false) = 0; 
WebKit/chromium/public/WebFrame.h:    virtual int printBegin(const WebSize& pageSize, int printerDPI = 72,
WebKit/chromium/public/WebFrame.h:                           bool* useBrowserOverlays = 0) = 0;
WebKit/chromium/public/WebInputEvent.h:    WebKeyboardEvent(unsigned sizeParam = sizeof(WebKeyboardEvent))
WebKit/chromium/public/WebInputEvent.h:    WebMouseEvent(unsigned sizeParam = sizeof(WebMouseEvent))
WebKit/chromium/public/WebInputEvent.h:    WebMouseWheelEvent(unsigned sizeParam = sizeof(WebMouseWheelEvent))
WebKit/chromium/public/WebInputEvent.h:    WebTouchEvent(unsigned sizeParam = sizeof(WebTouchEvent))
WebKit/chromium/public/WebVector.h:    explicit WebVector(size_t size = 0)

It might be ok to use default argument though Chromium style doesn't allow it.  I'm not 100% sure.

[Ilya Sherman]

(In reply to comment #5)
> (From update of attachment 71631 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=71631&action=review
> 
> > WebKit/chromium/public/WebInputElement.h:69
> > +        WEBKIT_API void setValue(const WebString&); // TODO(isherman): Remove once chromium code is updated.
> 
> We use some default arguments in WebKit API.
> It might be ok to use default argument though Chromium style doesn't allow it.  I'm not 100% sure.

I'm not really opposed to using a default argument here -- I was just trying to stay consistent with Chromium style =)

What say ye, wise guardians of the Chromium WebKit API?

[Darin Fisher (:fishd, Google)]

Comment on attachment 71631 [details]
Patch

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

> WebKit/chromium/public/WebInputElement.h:70
> +        WEBKIT_API void setValue(const WebString&, bool);

yes, please use a default argument.  also, please give this "bool" parameter a name.

[Ilya Sherman]

Created attachment 72547 [details]
Addresses Darin's review comments

Done and done =)

[Kent Tamura]

Comment on attachment 72547 [details]
Addresses Darin's review comments

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

> WebKit/chromium/ChangeLog:5
> +        Expose |sendChangeEvent| parameter in WebInputElement::setValue() API,

nit: we don't enclose a parameter name with || in WebKit style.

[Ilya Sherman]

Created attachment 72567 [details]
Patch

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 72567 [details]:

http/tests/appcache/fail-on-update-2.html
animations/suspend-resume-animation.html

Please file bugs against the tests.  These tests were authored by [e-mail] and [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 72567 [details]
Patch

Clearing flags on attachment: 72567

Committed r71158: <http://trac.webkit.org/changeset/71158>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.