Closed Bug 1408129 Opened 7 years ago Closed 6 years ago

commands API is not treated as user input

Categories

(WebExtensions :: General, enhancement, P5)

enhancement

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: wbamberg, Assigned: evilpie, Mentored)

References

Details

(Keywords: dev-doc-complete, good-first-bug)

Attachments

(2 files)

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.
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?
Severity: normal → enhancement
Priority: -- → P5
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.
Component: WebExtensions: Untriaged → WebExtensions: General
Blocks: 1438465
This seems to have broken the uppity extension: https://github.com/arantius/uppity/issues/3
Mentor: aswan
Keywords: good-first-bug
Product: Toolkit → WebExtensions
Assignee: nobody → evilpies
Attachment #8996157 - Flags: review?(aswan)
Attachment #8996158 - Flags: review?(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
Attachment #8996158 - Flags: review?(aswan) → review+
Comment on attachment 8996157 [details] [diff] [review] Treat webextension commands as user input. r? Review of attachment 8996157 [details] [diff] [review]: ----------------------------------------------------------------- thanks!
Attachment #8996157 - Flags: review?(aswan) → review+
Keywords: dev-doc-needed
Pushed by evilpies@gmail.com: 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
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
Flags: needinfo?(evilpies)
I am not sure what is going and I don't have a Mac for testing. Hopefully Andrew can figure it out.
Flags: needinfo?(evilpies) → needinfo?(aswan)
Pushed by evilpies@gmail.com: 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
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Flags: needinfo?(aswan)
Is manual QA needed on this bug? If yes, please provide a test webextension and some STRs, else add the "qe-verify-" flag.
Flags: needinfo?(evilpies)
No, thank you. I believe the automated test is good enough for this.
Flags: needinfo?(evilpies)
Flags: qe-verify-
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!
Flags: needinfo?(evilpies)
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.
Flags: needinfo?(evilpies)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: