1408129 - commands API is not treated as user input

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P5)

Description

[Will Bamberg [:wbamberg]]

Some APIs, like downloads.open, sidebarAction.open, browserAction.openPopup, are supposed to only work as a response to a user gesture. This includes things like clicking a browser action button or a context menu item.

It should also include pressing a keyboard shortcut defined using the commands API, but it doesn't.

Comment 1

[Andy McKay]

There specific code in commands to cause these actions to be enabled, bug 1338727 comes to mind. Presumably this bug is a request to execute code in a function from the commands API and then allow those APIs to be triggered... as opposed to directly mapping the commands API in the manifest?

Comment 2

[Will Bamberg [:wbamberg]]

Yes, that is the request. It seems inconsistent to have an API that requires a user action, then disallow something that's obviously a user action.

I'd forgotten about the special commands, and that certainly makes this matter less. Still, that doesn't help with downloads.open.

Comment 5

[Paul Wise]

This seems to have broken the uppity extension:

https://github.com/arantius/uppity/issues/3

Comment 7

[Tom S [:evilpie]]

Attachment: Treat webextension commands as user input. r?

Comment 8

[Tom S [:evilpie]]

Attachment: Remove dead references to InputEventManager. r?

Comment 9

[Andrew Swan [:aswan]]

Comment on attachment 8996158 [details] [diff] [review]
Remove dead references to InputEventManager. r?

Review of attachment 8996158 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for catching that

Comment 10

[Andrew Swan [:aswan]]

Comment on attachment 8996157 [details] [diff] [review]
Treat webextension commands as user input. r?

Review of attachment 8996157 [details] [diff] [review]:
-----------------------------------------------------------------

thanks!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7379720ca6f5
Treat webextension commands as user input. r=aswan
https://hg.mozilla.org/integration/mozilla-inbound/rev/d25226ca3633
Remove dead references to InputEventManager. r=aswan

Comment 12

[Cristian Brindusan [:cbrindusan]]

Backed out 2 changesets (Bug 1408129) for browser-chrome failures on /browser/test-oop-extensions/browser_ext_user_events.js. 

https://hg.mozilla.org/integration/mozilla-inbound/rev/d81a30b5a964862c5e8f43a57ed7964fad3c59e7

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=d25226ca3633ec13b0d79cce0303622678f025dc&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=191167566

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=191167566&repo=mozilla-inbound&lineNumber=2853

09:48:40     INFO - TEST-START | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js
09:48:42     INFO - TEST-INFO | started process screencapture
09:48:42     INFO - TEST-INFO | screencapture: exit 0
09:48:42     INFO - Buffered messages logged at 09:48:40
09:48:42     INFO - Entering test bound testSources
09:48:42     INFO - Extension loaded
09:48:42     INFO - Console message: Warning: attempting to write 28213 bytes to preference extensions.webextensions.uuids. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes.
09:48:42     INFO - Console message: Warning: attempting to write 5675 bytes to preference browser.pageActions.persistedActions. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes.
09:48:42     INFO - Buffered messages logged at 09:48:41
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | addon-webext-permissions notification shown - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | notification panel open - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | request() did not throw when called from page action click - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | request() succeeded when called from page action click - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | Expect widget not to be overflowed - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | addon-webext-permissions notification shown - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | notification panel open - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | request() did not throw when called from browser action click - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | request() succeeded when called from browser action click - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | Found context menu item - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | addon-webext-permissions notification shown - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | notification panel open - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | request() did not throw when called from context menu click - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | request() succeeded when called from context menu click - 
09:48:42     INFO - Buffered messages logged at 09:48:42
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | request() did not throw when called from options page link click - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | request() succeeded when called from options page link click - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | addon-webext-permissions notification shown - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | notification panel open - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | addon-webext-permissions notification shown - 
09:48:42     INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | notification panel open - 
09:48:42     INFO - Buffered messages finished
09:48:42     INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_user_events.js | uncaught exception - uncaught exception: PopupNotifications._onButtonEvent: couldn't find notification at undefined

Comment 13

[Tom S [:evilpie]]

I am not sure what is going and I don't have a Mac for testing. Hopefully Andrew can figure it out.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e89fb659271c
Treat webextension commands as user input. r=aswan
https://hg.mozilla.org/integration/mozilla-inbound/rev/253e2e30634e
Remove dead references to InputEventManager. r=aswan

Comment 15

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/e89fb659271c
https://hg.mozilla.org/mozilla-central/rev/253e2e30634e

Comment 16

[Victor Carciu]

Is manual QA needed on this bug? If yes, please provide a test webextension and some STRs, else add the "qe-verify-" flag.

Comment 17

[Tom S [:evilpie]]

No, thank you. I believe the automated test is good enough for this.

Comment 18

[Will Bamberg [:wbamberg]]

I thought it would be good to have a separate page on the content of "user actions" or "user input handlers", since there are a few places in the docs where we mention it, but we never really explain it.

https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/User_actions

Please let me know if you think this makes sense and covers the main points. I'm not too keen on listing all the APIs that have this restriction in this page, since it will tend to get out of date, but maybe we should do it anyway.

Thanks!

Comment 19

[Tom S [:evilpie]]

This is a great idea! I think the negative examples are especially worthwhile. https://searchfox.org/mozilla-central/search?q=requireUserInput&case=false&regexp=false&path=.json should give you a list of all APIs that require user input. We should update all the documentation for those APIs to point to that page.

Comment 20

[Will Bamberg [:wbamberg]]

Thanks :evilpie. I've updated all those pages to link to 
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/User_actions


https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browserAction/openPopup
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/pageAction/openPopup
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/search/search
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/sidebarAction/open
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/sidebarAction/close
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/downloads/open
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/permissions/request

...except the page for management.install(), because that doesn't exist yet (https://bugzilla.mozilla.org/show_bug.cgi?id=1369209).

I've also updated the compat data for commands.onCommand to note that it's only treated as a user input handler from 63 onwards: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/commands/onCommand#Browser_compatibility.

Marking as dev-doc-complete, but please let me know if you see anything else that's needed here.