Bug 102320 – HTMLFormElement#requestAutocomplete() should require a user action

• Component: Forms
• Status: RESOLVED FIXED

[Elliott Sprehn]

requestAutocomplete should require a user action and should dispatch a failure event when triggered from a non user action:

ex.

setTimeout(function() {
  form.requestAutocomplete(); // this should always fail.
}, 0);

[Adam Barth]

Should be an easy patch.

[Dan Beam]

If I'm just passing along this info to the embedder (so it can fail if it'd like), I guess I have to wait for the code to land in Chrome before I can test firing an autocompleteerror on a non-user gesture.  Also, is there a way to simulate user gestures in layout tests?

[Dan Beam]

Hey Ian and Adam,

This bug is about requiring HTMLFormElement#requestAutocomplete() be based on a user action, else dispatching an error.  Is this behavior going to be specified (meaning all WebKit implementations will need to behave the same) or will it be up to each embedder/browser to choose this behavior?

Whether it's standardized or not affects my patch, that's why I'm asking.

[Dan Beam]

Created attachment 177887 [details]
Patch

[Elliott Sprehn]

(In reply to comment #3)
> Hey Ian and Adam,
> 
> This bug is about requiring HTMLFormElement#requestAutocomplete() be based on a user action, else dispatching an error.  Is this behavior going to be specified (meaning all WebKit implementations will need to behave the same) or will it be up to each embedder/browser to choose this behavior?
> 
> Whether it's standardized or not affects my patch, that's why I'm asking.

As the person writing the preliminary spec I was going to require it for all implementations.

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Dan Beam]

Created attachment 177888 [details]
changelog typo

[Dan Beam]

esprehn@: I only see 2 differentiations on whether something is a user action or not in this document (http://dev.w3.org/html5/spec/single-page.html)

1) http://dev.w3.org/html5/spec/single-page.html#history-notes

"""
In addition, a user agent could ignore calls to pushState() that are invoked on a timer, or from event listeners that are not triggered in response to a clear user action, or that are invoked in rapid succession.
"""

2) http://dev.w3.org/html5/spec/single-page.html#context-menus

"""
The context information of the event must be initialized to the same values as the last MouseEvent user interaction event that was fired as part of the gesture that that was interpreted as a request for the context menu.
"""

It'd seem only #1 is applicable, and it's only a recommendation (if that).  However, there's a first time for everything, :).

[Dan Beam]

Created attachment 177894 [details]
if specified impl

[Kent Tamura]

Comment on attachment 177894 [details]
if specified impl

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

> Source/WebCore/ChangeLog:11
> +        No new tests (OOPS!).

OOPS!
You had better mention that you updated form-request-autocomplete.html.

[Dan Beam]

Comment on attachment 177894 [details]
if specified impl

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

>> Source/WebCore/ChangeLog:11
>> +        No new tests (OOPS!).
> 
> OOPS!
> You had better mention that you updated form-request-autocomplete.html.

Yeah, I don't know why the scripts aren't picking this up automatically... will update now.

[Elliott Sprehn]

(In reply to comment #8)
> esprehn@: I only see 2 differentiations on whether something is a user action or not in this document (http://dev.w3.org/html5/spec/single-page.html)

http://www.whatwg.org/specs/web-apps/current-work/#allowed-to-show-a-pop-up

window.open() is indeed spec'ed to require a trusted event. See also DOMWindow::allowPopUp which has the user gesture code baked into webkit, not into the embedder.

[Dan Beam]

Created attachment 177896 [details]
if unspecified impl

[Dan Beam]

Created attachment 177898 [details]
if specified impl

[Dan Beam]

(In reply to comment #13)
> Created an attachment (id=177896) [details]
> if unspecified impl

I talked with esprehn@ and we agreed that we'd want to spec the safer, simpler to implement version of this API.  So, I've obsoleted the unspecified implementation in favor of the specified one.

tkent@: if I could steal a minute of your time it'd be awesome to get a [very short] review on the non-obsolete patch (https://bugs.webkit.org/attachment.cgi?id=177898&action=review) now that I'm done juggling this bug's attachments, :P.

[Kent Tamura]

Comment on attachment 177898 [details]
if specified impl

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

> Source/WebCore/ChangeLog:14
> +        * fast/forms/form-request-autocomplete.html:
> +        
> +        Added a test to ensure that dispatching a call to HTMLFormElement#requestAutocomplete() in a setTimeout() (which will
> +        never be a user gesture) always fails.

nit: This file is not a part of WebCore. So you don't need to follow the file list style here.
"fast/forms/form-request-autocomplete.html is updated." is enough.

[Adam Barth]

> As the person writing the preliminary spec I was going to require it for all implementations.

Makes sense.

[Ian 'Hixie' Hickson]

I think it makes sense that it happen that way, but in practice the spec won't require it, since it's UI. But acting as if it does is appropriate.

[Dan Beam]

Created attachment 177925 [details]
Patch for landing

[Dan Beam]

Comment on attachment 177898 [details]
if specified impl

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

>> Source/WebCore/ChangeLog:14
>> +        never be a user gesture) always fails.
> 
> nit: This file is not a part of WebCore. So you don't need to follow the file list style here.
> "fast/forms/form-request-autocomplete.html is updated." is enough.

Done.

[Kent Tamura]

Comment on attachment 177925 [details]
Patch for landing

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

> Source/WebCore/ChangeLog:11
> +        * fast/forms/form-request-autocomplete.html is updated.

"* " is confusing.  It looks like a part of WebCore files.

[Dan Beam]

Created attachment 177931 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 177925 [details]
Patch for landing

Clearing flags on attachment: 177925

Committed r136800: <http://trac.webkit.org/changeset/136800>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dan Beam]

Comment on attachment 177925 [details]
Patch for landing

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

>> Source/WebCore/ChangeLog:11
>> +        * fast/forms/form-request-autocomplete.html is updated.
> 
> "* " is confusing.  It looks like a part of WebCore files.

Sorry, removed.

[Kent Tamura]

Comment on attachment 177931 [details]
Patch for landing

OMG

[Dan Beam]

(In reply to comment #24)
> All reviewed patches have been landed.  Closing bug.

Whoops, too late.  I'll remember next time.  I understand now, looking at the patchset (http://trac.webkit.org/changeset/136800), why * matters.

[Dan Beam]

(In reply to comment #26)
> (From update of attachment 177931 [details])
> OMG

Yes, bad timing :(.  Is there anything else I need to do for this bug?

[Dan Beam]

Comment on attachment 177925 [details]
Patch for landing

this was the committed patch