Closed Bug 1334096 Opened 7 years ago Closed 7 years ago

Sideloading button should be removed if it not used to enable to extension

Categories

(Toolkit :: Add-ons Manager, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla54
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- affected
firefox54 --- verified

People

(Reporter: vtamas, Assigned: aswan)

References

Details

(Whiteboard: triaged)

Attachments

(2 files)

Attached file webext123-1-an+fx.xpi
[Note]
This is a follow-up bug for Bug 1317363

[Affected versions]:
Firefox 53.0a2 (2017-01-25)
Firefox 54.0a1 (2017-01-25)

[Affected platforms]:
Windows 10 64-bit
Ubuntu 16.04 32-bit


[Steps to reproduce]:
1.Launch Firefox with clean profile.
2.Create extensions.webextPermissionPrompts and set it to true.
3.Create xpinstall.signatures.dev-root and set it to true.
4.Install via sideloading method the attached webextension. 
5.After reopening the browser, navigate to about:add-ons and enable the extension.


[Expected Results]:
Sideloading button should disappear from Panel Menu if the webextension was enabled from Add-ons Manager by skipping the sideloading flow.


[Actual Results]:
 - Sideloading button is still available, even the add-on was already enabled from Addons Manager.
 - Clicking the Sideloading button from Panel Menu will display the installation pop-up that offers the option to enable a webextension which is already active
 - See screencast: https://www.screencast.com/t/kn9Wd8rT
Blocks: webext-permissions
No longer blocks: 1317363
Assignee: nobody → aswan
Priority: -- → P1
Whiteboard: triaged
Ugh, I wrote the patches for this and then it occurs to me that if a user does this (finds a side-loaded extension in about:addons and enables it from there) then they never see the permissions list.  The options I can think of, sorted by increasing amount of work to implement are:
1. Do nothing, people rarely visit about:addons
2. Don't offer the "Enable" button in about:addons for a side-loaded extension.
3. Display the permission prompt every time a user clicks "Enable" on a webextension.
4. Display the permission prompt when a user clicks "Enable" for the first time on a side-loaded webextension.
(In reply to Andrew Swan [:aswan] from comment #1)
> Ugh, I wrote the patches for this and then it occurs to me that if a user
> does this (finds a side-loaded extension in about:addons and enables it from
> there) then they never see the permissions list.  The options I can think
> of, sorted by increasing amount of work to implement are:
> 1. Do nothing, people rarely visit about:addons
> 2. Don't offer the "Enable" button in about:addons for a side-loaded
> extension.
> 3. Display the permission prompt every time a user clicks "Enable" on a
> webextension.
> 4. Display the permission prompt when a user clicks "Enable" for the first
> time on a side-loaded webextension.

4 would be my choice, but I'm concious that its the most work.

I think if you do 2, you get stuck in a rabbit hole of ...why is there a disable button, but no enable button and then...
Thinking about it more, 4 shouldn't be too difficult, let me take a stab at it and if I run into some new issue we can revisit.
This makes me concerned though, is there any other place in the UI where one could enable a disabled extension?  (I can't think of any but there are other places like the "slow add-on" notification that let you disable extensions so I have this nagging paranoia that there's some obscure case that I'm overlooking...)
I thought of the gcli: https://developer.mozilla.org/en/docs/Tools/GCLI but I'm going to propose we just turn that feature off. I'll file a bug.
Attachment #8838374 - Flags: review?(dtownsend)
Comment on attachment 8838374 [details]
Bug 1334096 Show permissions prompts when a sideloaded extension is enabled

https://reviewboard.mozilla.org/r/113310/#review114998

I think you've covered the case already but it would be nice to see an additional test for going to the details view of the add-on and enabling from there. Looks good otherwise.
Attachment #8838374 - Flags: review?(dtownsend)
Comment on attachment 8838374 [details]
Bug 1334096 Show permissions prompts when a sideloaded extension is enabled

https://reviewboard.mozilla.org/r/113310/#review115626
Attachment #8838374 - Flags: review?(dtownsend) → review+
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ae43afe8e551
Show permissions prompts when a sideloaded extension is enabled r=mossop
Blocks: 1342142
Backed out for failing browser_CTP_plugins.js tests:

https://hg.mozilla.org/integration/autoland/rev/120f133678392553fbe45367d91a5252b5d7052b

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ae43afe8e5514c430a2329293b565afaac1f5886&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log example: https://treeherder.mozilla.org/logviewer.html#?job_id=79698519&repo=autoland

[task 2017-02-23T17:43:07.853369Z] 17:43:07     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/test-window/browser_CTP_plugins.js | Element should not be null, when checking visibility - 
[task 2017-02-23T17:43:07.855316Z] 17:43:07     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/test-window/browser_CTP_plugins.js | part8: detail state menu should be visible - 
[task 2017-02-23T17:43:07.858220Z] 17:43:07     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/test-window/browser_CTP_plugins.js | part8: state menu should have 'Never Activate' selected - 
[task 2017-02-23T17:43:07.861189Z] 17:43:07     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/test-window/browser_CTP_plugins.js | part9: should have a plugin element in the page - {} == true - 
[task 2017-02-23T17:43:07.862928Z] 17:43:07     INFO - TEST-PASS | toolkit/mozapps/extensions/test/browser/test-window/browser_CTP_plugins.js | part10: plugin should be activated - true == true - 
[task 2017-02-23T17:43:07.864823Z] 17:43:07     INFO - Buffered messages finished
[task 2017-02-23T17:43:07.867690Z] 17:43:07     INFO - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/test-window/browser_CTP_plugins.js | Uncaught exception - part11: should have a click-to-play notification - timed out after 50 tries.
[task 2017-02-23T17:43:07.869327Z] 17:43:07     INFO - Leaving test bound 
[task 2017-02-23T17:43:07.871082Z] 17:43:07     INFO - Not taking screenshot here: see the one that was previously logged
[task 2017-02-23T17:43:07.873813Z] 17:43:07     INFO - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/test-window/browser_CTP_plugins.js | Found unexpected Addons:Manager window still open - 
[task 2017-02-23T17:43:07.875302Z] 17:43:07     INFO - Stack trace:
[task 2017-02-23T17:43:07.877199Z] 17:43:07     INFO -     chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/test-window/head.js:checkOpenWindows:120
[task 2017-02-23T17:43:07.879422Z] 17:43:07     INFO -     chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/test-window/head.js:null:172
[task 2017-02-23T17:43:07.881131Z] 17:43:07     INFO -     chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest<:437
[task 2017-02-23T17:43:07.882899Z] 17:43:07     INFO -     testScope/test_finish/<@chrome://mochikit/content/browser-test.js:1006:11
[task 2017-02-23T17:43:07.884981Z] 17:43:07     INFO -     run@chrome://mochikit/content/browser-test.js:937:9
Flags: needinfo?(aswan)
Doh, plugins.  Patch updated but running it through try (and paying closer attention to the results this time) before re-landing.
Flags: needinfo?(aswan)
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fae64acfaddc
Show permissions prompts when a sideloaded extension is enabled r=mossop
https://hg.mozilla.org/mozilla-central/rev/fae64acfaddc
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Verified this fix on Firefox 54.0a1 (2017-02-27) under Windows 10 64-bit, Ubuntu 16.04 32-bit and Mac OS X 10.12.1. 
During testing I’ve encountered 2 follow-up bugs: Bug 1343179 and Bug 1343222

I will mark this bug as Verified, since the other issues are tracked separately.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.