commands API is not treated as user input

RESOLVED FIXED in Firefox 63

Status

P5
enhancement
RESOLVED FIXED
2 years ago
7 months ago

People

(Reporter: wbamberg, Assigned: evilpie, Mentored)

Tracking

({dev-doc-complete, good-first-bug})

unspecified
mozilla63
dev-doc-complete, good-first-bug
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
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

2 years ago
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
(Reporter)

Comment 2

2 years ago
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.
Duplicate of this bug: 1416603
Duplicate of this bug: 1433167
Component: WebExtensions: Untriaged → WebExtensions: General

Comment 5

a year ago
This seems to have broken the uppity extension:

https://github.com/arantius/uppity/issues/3
Duplicate of this bug: 1443285
Mentor: aswan
Keywords: good-first-bug

Updated

9 months ago
Product: Toolkit → WebExtensions
(Assignee)

Updated

8 months ago
Assignee: nobody → evilpies
(Assignee)

Updated

8 months ago
Attachment #8996157 - Flags: review?(aswan)
(Assignee)

Updated

8 months ago
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+
(Assignee)

Updated

8 months ago
Keywords: dev-doc-needed

Comment 11

8 months ago
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)
(Assignee)

Comment 13

8 months ago
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)

Comment 14

8 months ago
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

Comment 15

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e89fb659271c
https://hg.mozilla.org/mozilla-central/rev/253e2e30634e
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63

Updated

7 months ago
Flags: needinfo?(aswan)

Comment 16

7 months ago
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)
(Assignee)

Comment 17

7 months ago
No, thank you. I believe the automated test is good enough for this.
Flags: needinfo?(evilpies)

Updated

7 months ago
Flags: qe-verify-
(Reporter)

Comment 18

7 months ago
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)
(Assignee)

Comment 19

7 months ago
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.