Closed Bug 1308295 Opened 8 years ago Closed 8 years ago

Prompt users with permissions for third-party webextensions installs

Categories

(WebExtensions :: General, defect, P2)

Product:
WebExtensions
Component:
General
Version:
51 Branch
Type:
defect

Tracking

(firefox53 verified)

Status:
VERIFIED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox53 --- verified

People

(Reporter: aswan, Assigned: aswan)

References

Details

(Whiteboard: [triaged][permissions])

Attachments

(3 files, 1 obsolete file)

Screenshot 2016-10-06 10.14.29.png
61.56 KB, image/png
Details
Bug 1308295 part 1: rename permHandler -> promptHandler
59 bytes, text/x-review-board-request
rhelmer
: review+
Details
Bug 1308295 part 2: add extension prompts to InstallTrigger
59 bytes, text/x-review-board-request
rhelmer
: review+
Details 
This is the "confirm installation & permissions" prompt from the third party flow detailed at https://www.figma.com/file/HrLiKUwoLQZsIUIVBKM8Wnnu/Install-Flow-showing-Permissions
Priority: -- → P2
Whiteboard: [triaged]
Depends on: 1004061
Whiteboard: [triaged] → [triaged][permissions]
This bug is about extensions installed using IntallTrigger.
I talked with Markus in Hawaii and he asked to keep the existing install confirmation dialog for non-webextensions and only display the permissions dialog for webextensions that require permissions.
But the problem is, InstallTrigger can be used to install multiple addons with a single call and the existing dialog is currently displayed once, listing all the addons being installed.  Which raises the question of what to do if multiple webextensions are being installed at once (or worse, some combination of webextensions and non-webextensions).  The simplest thing would be to change InstallTrigger to only let it install one thing at a time, there was some chatter about this on irc a week or two ago.  Dave and Rob, any opinions on that?  If we decide against changing the api, I'll bounce this over to Markus for a decision on how to handle the prompts.
Assignee: nobody → aswan
Flags: needinfo?(rhelmer)
Flags: needinfo?(dtownsend)
I think we should drop the multiple install scenarios, both multi-package and being able to pass multiple extensions to InstallTrigger, it would help clean up a bunch of the code and reduce attack surfaces. But that's definitely something to float on the mailing list first to see if people are still using it for something more useful than we know of.
Flags: needinfo?(dtownsend)
Agree with Mossop in comment 3.

Multi-package is useful right now to bundle a theme and an extension, but there won't be a distinction between extensions and themes in WebExtensions.
Flags: needinfo?(rhelmer)
Attachment #8822743 - Attachment is obsolete: true
Comment on attachment 8822741 [details]
Bug 1308295 part 1: rename permHandler -> promptHandler

https://reviewboard.mozilla.org/r/101546/#review103154
Attachment #8822741 - Flags: review?(rhelmer) → review+
Comment on attachment 8822742 [details]
Bug 1308295 part 2: add extension prompts to InstallTrigger

https://reviewboard.mozilla.org/r/101548/#review103322

::: browser/modules/ExtensionsUI.jsm:25
(Diff revision 3)
>  
>    observe(subject, topic, data) {
>      if (topic == "webextension-permission-prompt") {
>        let {target, info} = subject.wrappedJSObject;
> +
> +      let progressNotification = target.ownerGlobal.PopupNotifications.getNotification("addon-progress", target);

Is it possible for the user to install 2 add-ons from the same page, first one that weights several MB and takes minutes to download, then a tiny webextension that will show the permission prompt almost immediately. If so, is it possible for this code to remove the progress bar of the first add-on unintentionally?
Comment on attachment 8822742 [details]
Bug 1308295 part 2: add extension prompts to InstallTrigger

https://reviewboard.mozilla.org/r/101548/#review103322

> Is it possible for the user to install 2 add-ons from the same page, first one that weights several MB and takes minutes to download, then a tiny webextension that will show the permission prompt almost immediately. If so, is it possible for this code to remove the progress bar of the first add-on unintentionally?

Hm, I don't think the existing code handles simultaneous installs very well at all.  From inspecting code (but not actually trying to create this situation), it looks like before we get to the permissions dialog, simply starting the second install will clobber the progress dialog from the first install.
(In reply to Andrew Swan [:aswan] from comment #13)

> Hm, I don't think the existing code handles simultaneous installs very well
> at all.

I know some add-on related code can show simultaneous notifications (otherwise we wouldn't need to fix bug 1319732), but I don't know exactly which parts of this work and which are already broken.
(In reply to Florian Quèze [:florian] [:flo] from comment #14)
> (In reply to Andrew Swan [:aswan] from comment #13)
> > Hm, I don't think the existing code handles simultaneous installs very well
> > at all.
> 
> I know some add-on related code can show simultaneous notifications
> (otherwise we wouldn't need to fix bug 1319732), but I don't know exactly
> which parts of this work and which are already broken.

Yeah, that bug covers the specific case where the confirmation notification for install 1 is displayed when the progress notification for install 2 comes along.  In the scenario you outlined previously, that doesn't happen -- the progress notification for install 1 is still present when install 2 starts.

I'm not sure what to do here, I suspect the problem is bigger than the few individual cases we've discussed.  I'm pretty sure this patch doesn't make the problem any worse.  I don't feel great about leaving this in this state but really doing something meaningful about it looks like a big job.
Comment on attachment 8822742 [details]
Bug 1308295 part 2: add extension prompts to InstallTrigger

https://reviewboard.mozilla.org/r/101548/#review103158

::: browser/base/content/test/general/browser_extension_permissions.js:13
(Diff revision 3)
>  const NO_PERMS_XPI = `${BASE}/browser_webext_nopermissions.xpi`;
>  const ID = "permissions@test.mozilla.org";
>  
>  const DEFAULT_EXTENSION_ICON = "chrome://browser/content/extension.svg";
>  
> +var pm = Services.perms;

Nitpick but why `var`? Also I know it's used twice below but repeating here doesn't seem especially eggregious:

`Services.perms.add(makeURI("https://example.com/"), "install", Services.perms.ALLOW_ACTION);`

Otherwise seems like this could be `const`.

::: browser/modules/ExtensionsUI.jsm:25
(Diff revision 3)
>  
>    observe(subject, topic, data) {
>      if (topic == "webextension-permission-prompt") {
>        let {target, info} = subject.wrappedJSObject;
> +
> +      let progressNotification = target.ownerGlobal.PopupNotifications.getNotification("addon-progress", target);

We discussed this in IRC and tested it, I agree that while it's a problem we should fix, it doesn't regress the current behavior. It's also not the only place that exhibits this.

I think we want to queue up these sort of prompts, so when one is dismissed the one "beneath" it is revealed, similar to the way infobars work. Let's file a separate bug for this though.
Attachment #8822742 - Flags: review?(rhelmer) → review+
Comment on attachment 8822742 [details]
Bug 1308295 part 2: add extension prompts to InstallTrigger

https://reviewboard.mozilla.org/r/101548/#review103322

> Hm, I don't think the existing code handles simultaneous installs very well at all.  From inspecting code (but not actually trying to create this situation), it looks like before we get to the permissions dialog, simply starting the second install will clobber the progress dialog from the first install.

Filed bug 1329884 to pick this up.  If Rob and Florian are both okay with it, I'd like to land this one independently of fixing that bug.
(In reply to Andrew Swan [:aswan] from comment #18)

> > Hm, I don't think the existing code handles simultaneous installs very well at all.  From inspecting code (but not actually trying to create this situation), it looks like before we get to the permissions dialog, simply starting the second install will clobber the progress dialog from the first install.
> 
> Filed bug 1329884 to pick this up.  If Rob and Florian are both okay with
> it, I'd like to land this one independently of fixing that bug.

Thanks for filing it. And yes, fixing that later is fine. I would just suggest adding a comment mentioning the bug number, so that the next person looking at it doesn't scratch his head wondering if it's correct.
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8191c9b20ca
part 1: rename permHandler -> promptHandler r=rhelmer
https://hg.mozilla.org/integration/autoland/rev/c0f94e1d6f11
part 2: add extension prompts to InstallTrigger r=rhelmer
(In reply to Florian Quèze [:florian] [:flo] from comment #19)
> I would just
> suggest adding a comment mentioning the bug number, so that the next person
> looking at it doesn't scratch his head wondering if it's correct.

Ack, sorry I saw this comment right after I pressed the "land" button.  I'll sneak the comment in in one of the subsequent patches.
Backed out for failing browser-chrome tests browser_bug567127.js, browser_discovery_install.js and browser_getmorethemes.js:

https://hg.mozilla.org/integration/autoland/rev/d2eccdc4a23256d51d2e32986a59b6ffadbafa19
https://hg.mozilla.org/integration/autoland/rev/1a7e3e649e7bbeba69c95b0f7ab67480e70d2948

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c0f94e1d6f111be24267f5d0e2ee5b09c31aa231

Failure log browser_bug567127.js: https://treeherder.mozilla.org/logviewer.html#?job_id=67664967&repo=autoland

[task 2017-01-10T16:32:57.518413Z] 16:32:57     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/browser_bug567127.js | Saw 2 addon-install-started notifications - 
[task 2017-01-10T16:32:57.521635Z] 16:32:57     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/browser_bug567127.js | Should have an add-ons manager window to close - 
[task 2017-01-10T16:32:57.523839Z] 16:32:57     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/browser_bug567127.js | Should be closing window with correct URI - 
[task 2017-01-10T16:32:57.526309Z] 16:32:57     INFO - Telling manager window to close
[task 2017-01-10T16:32:57.530375Z] 16:32:57     INFO - Manager window close() call returned
[task 2017-01-10T16:32:57.532239Z] 16:32:57     INFO - Buffered messages finished
[task 2017-01-10T16:32:57.534340Z] 16:32:57     INFO - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_bug567127.js | A promise chain failed to handle a rejection:  - undefined
[task 2017-01-10T16:32:57.536633Z] 16:32:57     INFO - Stack trace:
[task 2017-01-10T16:32:57.538596Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: register :: line 194
[task 2017-01-10T16:32:57.540427Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: completePromise :: line 706
[task 2017-01-10T16:32:57.542189Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/AddonManager.jsm :: reject :: line 2793
[task 2017-01-10T16:32:57.543782Z] 16:32:57     INFO -     JS frame :: chrome://mozapps/content/xpinstall/xpinstallConfirm.js :: myUnload :: line 158
[task 2017-01-10T16:32:57.545542Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/AddonManager.jsm :: setupPromptHandler/install.promptHandler/< :: line 2885
[task 2017-01-10T16:32:57.547212Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: Promise :: line 385
[task 2017-01-10T16:32:57.548940Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/AddonManager.jsm :: setupPromptHandler/install.promptHandler :: line 2789
[task 2017-01-10T16:32:57.550670Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: checkPrompt/< :: line 5633
[task 2017-01-10T16:32:57.552467Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/Task.jsm :: TaskImpl_run :: line 319
[task 2017-01-10T16:32:57.554451Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/Task.jsm :: TaskImpl :: line 277
[task 2017-01-10T16:32:57.556162Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/Task.jsm :: asyncFunction :: line 252
[task 2017-01-10T16:32:57.557743Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/Task.jsm :: Task_spawn :: line 166
[task 2017-01-10T16:32:57.559381Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: checkPrompt :: line 5624
[task 2017-01-10T16:32:57.561058Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: install :: line 5355
[task 2017-01-10T16:32:57.562759Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: install :: line 6066
[task 2017-01-10T16:32:57.564331Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: install :: line 6616
[task 2017-01-10T16:32:57.565862Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/AddonManager.jsm :: startInstall :: line 2087
[task 2017-01-10T16:32:57.567457Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/AddonManager.jsm :: installAddonFromAOM :: line 2192
[task 2017-01-10T16:32:57.569060Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/AddonManager.jsm :: installAddonFromAOM :: line 3587
[task 2017-01-10T16:32:57.570827Z] 16:32:57     INFO -     JS frame :: chrome://mozapps/content/extensions/extensions.js :: doCommand/< :: line 1353
[task 2017-01-10T16:32:57.572707Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/AddonManager.jsm :: safeCall :: line 196
[task 2017-01-10T16:32:57.574331Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/AddonManager.jsm :: makeSafe/< :: line 211
[task 2017-01-10T16:32:57.575957Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: process :: line 917
[task 2017-01-10T16:32:57.577599Z] 16:32:57     INFO -     JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: walkerLoop :: line 801
[task 2017-01-10T16:32:57.579178Z] 16:32:57     INFO -     Tester_execTest@chrome://mochikit/content/browser-test.js:737:9
[task 2017-01-10T16:32:57.580718Z] 16:32:57     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:657:7
[task 2017-01-10T16:32:57.582339Z] 16:32:57     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59

Failure log other failures: https://treeherder.mozilla.org/logviewer.html#?job_id=67664975&repo=autoland

[task 2017-01-10T16:26:18.150594Z] 16:26:18     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/browser_discovery_install.js | Should have an add-ons manager window - 
[task 2017-01-10T16:26:18.153187Z] 16:26:18     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/browser_discovery_install.js | Should be displaying the correct UI - 
[task 2017-01-10T16:26:18.153383Z] 16:26:18     INFO - must wait for load
[task 2017-01-10T16:26:18.153731Z] 16:26:18     INFO - window has focus, waiting for manager load
[task 2017-01-10T16:26:18.161528Z] 16:26:18     INFO - Manager waiting for view load
[task 2017-01-10T16:26:18.164775Z] 16:26:18     INFO - Console message: 1484065576851	addons.xpi	DEBUG	Download started for https://example.com/browser/toolkit/mozapps/extensions/test/xpinstall/amosigned.xpi to file /tmp/tmp-a7r.xpi
[task 2017-01-10T16:26:18.166682Z] 16:26:18     INFO - Console message: 1484065576854	addons.xpi	DEBUG	Download of https://example.com/browser/toolkit/mozapps/extensions/test/xpinstall/amosigned.xpi completed.
[task 2017-01-10T16:26:18.168546Z] 16:26:18     INFO - Console message: 1484065576912	addons.repository	DEBUG	cacheAddons: enabled true IDs ["unsigned-xpi@tests.mozilla.org"]
[task 2017-01-10T16:26:18.174805Z] 16:26:18     INFO - Console message: 1484065576927	addons.repository	DEBUG	Requesting http://127.0.0.1:8888/extensions-dummy/repositoryGetURL
[task 2017-01-10T16:26:18.176501Z] 16:26:18     INFO - Buffered messages finished
[task 2017-01-10T16:26:18.178951Z] 16:26:18     INFO - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_discovery_install.js | uncaught exception - TypeError: tabForEvent is undefined at onxblDOMWillOpenModalDialog@chrome://browser/content/tabbrowser.xml:5188:1
[task 2017-01-10T16:26:18.180392Z] 16:26:18     INFO - fireDialogEvent@resource://gre/modules/SharedPromptUtils.jsm:28:9
[task 2017-01-10T16:26:18.181826Z] 16:26:18     INFO - setupPromptHandler/install.promptHandler/<@resource://gre/modules/AddonManager.jsm:2883:13
[task 2017-01-10T16:26:18.183344Z] 16:26:18     INFO - Promise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:385:5
[task 2017-01-10T16:26:18.184821Z] 16:26:18     INFO - setupPromptHandler/install.promptHandler@resource://gre/modules/AddonManager.jsm:2789:37
[task 2017-01-10T16:26:18.186240Z] 16:26:18     INFO - checkPrompt/<@resource://gre/modules/addons/XPIProvider.jsm:5633:17
[task 2017-01-10T16:26:18.187621Z] 16:26:18     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:319:42
[task 2017-01-10T16:26:18.188851Z] 16:26:18     INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3
[task 2017-01-10T16:26:18.190338Z] 16:26:18     INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14
[task 2017-01-10T16:26:18.192292Z] 16:26:18     INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12
[task 2017-01-10T16:26:18.198824Z] 16:26:18     INFO - checkPrompt@resource://gre/modules/addons/XPIProvider.jsm:5624:5
[task 2017-01-10T16:26:18.201017Z] 16:26:18     INFO - install@resource://gre/modules/addons/XPIProvider.jsm:5355:7
[task 2017-01-10T16:26:18.202383Z] 16:26:18     INFO - install@resource://gre/modules/addons/XPIProvider.jsm:6131:7
[task 2017-01-10T16:26:18.203857Z] 16:26:18     INFO - downloadCompleted/<@resource://gre/modules/addons/XPIProvider.jsm:6447:9
[task 2017-01-10T16:26:18.205675Z] 16:26:18     INFO - makeSafe/<@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:171:17
[task 2017-01-10T16:26:18.207526Z] 16:26:18     INFO - getRepositoryAddon@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:152:5
[task 2017-01-10T16:26:18.209398Z] 16:26:18     INFO - getAddon/<@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:1068:9
[task 2017-01-10T16:26:18.211188Z] 16:26:18     INFO - process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:917:23
[task 2017-01-10T16:26:18.213034Z] 16:26:18     INFO - walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:801:7
[task 2017-01-10T16:26:18.214650Z] 16:26:18     INFO - Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:734:11
[task 2017-01-10T16:26:18.216087Z] 16:26:18     INFO - schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:765:7
[task 2017-01-10T16:26:18.217691Z] 16:26:18     INFO - completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:702:7
[task 2017-01-10T16:26:18.219054Z] 16:26:18     INFO - searchFailed@resource://gre/modules/addons/AddonRepository.jsm:685:13
[task 2017-01-10T16:26:18.220526Z] 16:26:18     INFO - _reportFailure@resource://gre/modules/addons/AddonRepository.jsm:957:5
[task 2017-01-10T16:26:18.221883Z] 16:26:18     INFO - _beginSearch/<@resource://gre/modules/addons/AddonRepository.jsm:1450:9
[task 2017-01-10T16:26:18.223347Z] 16:26:18     INFO - EventListener.handleEvent*_beginSearch@resource://gre/modules/addons/AddonRepository.jsm:1443:5
[task 2017-01-10T16:26:18.224987Z] 16:26:18     INFO - _beginGetAddons@resource://gre/modules/addons/AddonRepository.jsm:859:5
[task 2017-01-10T16:26:18.226872Z] 16:26:18     INFO - getAddonsByIDs@resource://gre/modules/addons/AddonRepository.jsm:755:12
[task 2017-01-10T16:26:18.228321Z] 16:26:18     INFO - cacheAddons/<@resource://gre/modules/addons/AddonRepository.jsm:675:7
[task 2017-01-10T16:26:18.229920Z] 16:26:18     INFO - process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:917:23
[task 2017-01-10T16:26:18.231421Z] 16:26:18     INFO - Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:734:11
[task 2017-01-10T16:26:18.233234Z] 16:26:18     INFO - schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:765:7
[task 2017-01-10T16:26:18.234767Z] 16:26:18     INFO - completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:702:7
[task 2017-01-10T16:26:18.236151Z] 16:26:18     INFO - openSignedAppFileFinished@resource://gre/modules/addons/XPIProvider.jsm:1819:9
[task 2017-01-10T16:26:18.237510Z] 16:26:18     INFO - 
[task 2017-01-10T16:26:18.238781Z] 16:26:18     INFO - Stack trace:
[task 2017-01-10T16:26:18.240232Z] 16:26:18     INFO -     chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:simpletestOnerror:1583
[task 2017-01-10T16:26:18.241715Z] 16:26:18     INFO -     resource://gre/modules/SharedPromptUtils.jsm:fireDialogEvent:28
[task 2017-01-10T16:26:18.246894Z] 16:26:18     INFO -     resource://gre/modules/AddonManager.jsm:setupPromptHandler/install.promptHandler/<:2883
[task 2017-01-10T16:26:18.249258Z] 16:26:18     INFO -     resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:Promise:385
[task 2017-01-10T16:26:18.252425Z] 16:26:18     INFO -     resource://gre/modules/AddonManager.jsm:setupPromptHandler/install.promptHandler:2789
[task 2017-01-10T16:26:18.254214Z] 16:26:18     INFO -     resource://gre/modules/addons/XPIProvider.jsm:checkPrompt/<:5633
[task 2017-01-10T16:26:18.255655Z] 16:26:18     INFO -     checkPrompt@resource://gre/modules/addons/XPIProvider.jsm:5624:5
[task 2017-01-10T16:26:18.257265Z] 16:26:18     INFO -     install@resource://gre/modules/addons/XPIProvider.jsm:5355:7
[task 2017-01-10T16:26:18.258930Z] 16:26:18     INFO -     install@resource://gre/modules/addons/XPIProvider.jsm:6131:7
[task 2017-01-10T16:26:18.260371Z] 16:26:18     INFO -     downloadCompleted/<@resource://gre/modules/addons/XPIProvider.jsm:6447:9
[task 2017-01-10T16:26:18.261930Z] 16:26:18     INFO -     makeSafe/<@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:171:17
[task 2017-01-10T16:26:18.263464Z] 16:26:18     INFO -     getRepositoryAddon@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:152:5
[task 2017-01-10T16:26:18.264971Z] 16:26:18     INFO -     getAddon/<@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:1068:9
[task 2017-01-10T16:26:18.266370Z] 16:26:18     INFO -     process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:917:23
[task 2017-01-10T16:26:18.267857Z] 16:26:18     INFO -     walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:801:7
[task 2017-01-10T16:26:18.269329Z] 16:26:18     INFO -     Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:734:11
[task 2017-01-10T16:26:18.270970Z] 16:26:18     INFO -     schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:765:7
[task 2017-01-10T16:26:18.272393Z] 16:26:18     INFO -     completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:702:7
[task 2017-01-10T16:26:18.274057Z] 16:26:18     INFO -     searchFailed@resource://gre/modules/addons/AddonRepository.jsm:685:13
[task 2017-01-10T16:26:18.275574Z] 16:26:18     INFO -     _reportFailure@resource://gre/modules/addons/AddonRepository.jsm:957:5
[task 2017-01-10T16:26:18.276951Z] 16:26:18     INFO -     _beginSearch/<@resource://gre/modules/addons/AddonRepository.jsm:1450:9
[task 2017-01-10T16:26:18.278593Z] 16:26:18     INFO -     EventListener.handleEvent*_beginSearch@resource://gre/modules/addons/AddonRepository.jsm:1443:5
[task 2017-01-10T16:26:18.280515Z] 16:26:18     INFO -     _beginGetAddons@resource://gre/modules/addons/AddonRepository.jsm:859:5
[task 2017-01-10T16:26:18.281915Z] 16:26:18     INFO -     getAddonsByIDs@resource://gre/modules/addons/AddonRepository.jsm:755:12
[task 2017-01-10T16:26:18.283443Z] 16:26:18     INFO -     cacheAddons/<@resource://gre/modules/addons/AddonRepository.jsm:675:7
[task 2017-01-10T16:26:18.285274Z] 16:26:18     INFO -     process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:917:23
[task 2017-01-10T16:26:18.291112Z] 16:26:18     INFO -     Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:734:11
[task 2017-01-10T16:26:18.293235Z] 16:26:18     INFO -     schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:765:7
[task 2017-01-10T16:26:18.295391Z] 16:26:18     INFO -     completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:702:7
[task 2017-01-10T16:26:18.297123Z] 16:26:18     INFO -     openSignedAppFileFinished@resource://gre/modules/addons/XPIProvider.jsm:1819:9
[task 2017-01-10T16:26:18.298929Z] 16:26:18     INFO - JavaScript error: chrome://browser/content/tabbrowser.xml, line 5188: TypeError: tabForEvent is undefined
Flags: needinfo?(aswan)
Whoops, try run for the original landing was carelessly copied and pasted and didn't cover the right tests.  Rebased and fixed tests, lets try this again.
Flags: needinfo?(aswan)
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1a8cdaa23310
part 1: rename permHandler -> promptHandler r=rhelmer
https://hg.mozilla.org/integration/autoland/rev/192faee59193
part 2: add extension prompts to InstallTrigger r=rhelmer
https://hg.mozilla.org/mozilla-central/rev/1a8cdaa23310
https://hg.mozilla.org/mozilla-central/rev/192faee59193
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Confirm that permissions are successfully prompted for third-party webextensions installs. Tested on Firefox 53.0a1 (2017-01-15) under Windows 10 64-bit, Ubuntu 16.04 32-bit and Mac OS X 10.12.1 while installing webextensions form https://people-mozilla.org/ 

Questions: 
  - Is addons.mozilla.org also considered a third-party site? The permissions are prompted while installing webextensions from amo - https://www.screencast.com/t/5rky6ZB5

  - Will be there any particular doohangers for unsigned webextensions (when the xpinstall.signatures.required is set to false) such as https://www.screencast.com/t/3e4wrpejp ? Because there is no “Caution” prompted while installing an unsigned webextension.
Flags: needinfo?(aswan)
No longer blocks: 1331618
Thanks for the quick testing!

(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #30)
>   - Is addons.mozilla.org also considered a third-party site? The
> permissions are prompted while installing webextensions from amo -
> https://www.screencast.com/t/5rky6ZB5

Yes, sorry for the confusing wording of the bug but this is really about anything that uses the InstallTrigger API, which includes AMO.

>   - Will be there any particular doohangers for unsigned webextensions (when
> the xpinstall.signatures.required is set to false) such as
> https://www.screencast.com/t/3e4wrpejp ? Because there is no “Caution”
> prompted while installing an unsigned webextension.

Oh, good catch.  Markus, the "unsigned" warning was part of the old confirmation dialog that the permission dialog has replaced.  How do you want to handle this?
Flags: needinfo?(aswan) → needinfo?(mjaritz)
(In reply to Andrew Swan [:aswan] from comment #31)
> (In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #30)
> >   - Will be there any particular doohangers for unsigned webextensions (when
> > the xpinstall.signatures.required is set to false) such as
> > https://www.screencast.com/t/3e4wrpejp ? Because there is no “Caution”
> > prompted while installing an unsigned webextension.
> 
> Oh, good catch.  Markus, the "unsigned" warning was part of the old
> confirmation dialog that the permission dialog has replaced.  How do you
> want to handle this?

Good point. Yes, please keep that warning about an extension being unsigned also as part of the new dialog! It is an edge-case but we should still tell the user that they are about to grant permissions to an unsigned extension.
I assume we might need to add another sentence to the permission doorhanger in that case. Maybe we can use the same text...
Emanuela, can you please make a proposal for that, building from the old warning and matching the new flow.
Flags: needinfo?(mjaritz) → needinfo?(emanuela)
Filed bug 1332061 for the follow-up to add the unsigned warning
Based on Comment 30 and Comment 31 I am marking this bug as Verified since the unsigned warning issue is tracked in a separate bug.
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified
Flags: needinfo?(emanuela)
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: