1495749 - Extensions.Uninstall action should clear the Install runOnce pref so that an extension can be reinstalled

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect, P3)

Description

[NN]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180920131237

Steps to reproduce:

Install extension using group policy in HKEY_LOCAL_MACHINE\SOFTWARE\Policies\Mozilla\Firefox\Extensions\Install 

Open Firefox.
See the extension installed.
In about:config the key "browser.policies.runOncePerModification.extensionsInstall" is set to the path of xpi.

Remove group policy registry key.
Run Firefox.
The extension is not removed
In about:config the value of browser.policies.runOncePerModification.extensionsInstall is not reset.

Same for HKEY_LOCAL_MACHINE\SOFTWARE\Policies\Mozilla\Firefox\Extensions\Uninstall
and key in about:config "browser.policies.runOncePerModification.extensionsUninstall" .
After adding extension to Uninstall, it won't be able to be installed once again due to the value which is not reset.


Actual results:

Cannot install and uninstall extensions using group policy


Expected results:

Install and uninstall extension using group policy should work as documented.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

The Install/Uninstall actions are a one-time action, so removing them from the registry is not meant to revert their effects (i.e., removing an extension from Install is not going to Uninstall the extension).

But what you mentioned about Uninstall not clearing the runOnce pref and thus not being able to reinstall the extension is definitely a bug, and we should fix it.

Comment 2

[NN]

Is there any reason to not work like Chrome does ?

Comment 3

[gabiganam]

(In reply to :Felipe Gomes (needinfo me!) from comment #1)
> The Install/Uninstall actions are a one-time action, so removing them from
> the registry is not meant to revert their effects (i.e., removing an
> extension from Install is not going to Uninstall the extension).
> 
> But what you mentioned about Uninstall not clearing the runOnce pref and
> thus not being able to reinstall the extension is definitely a bug, and we
> should fix it.

Hi, please note that while removing the install registry key isn't supposed to remove the extension like you say - it too does not clear the runOnce pref, and thus you won't be able to install the extension again, even if the uninstall runOnce pref is removed.
So the clearing of the runOnce pref should be fixed for both install and uninstall registry keys.

Comment 4

[gabiganam]

Also note that the defect exists & should be fixed in the ESR version.

Comment 5

[gabiganam]

@Felipe, any updates on this defect?
Planned fix? ETA?

Comment 6

[gabiganam]

(In reply to :Felipe Gomes (needinfo me!) from comment #1)
> The Install/Uninstall actions are a one-time action, so removing them from
> the registry is not meant to revert their effects (i.e., removing an
> extension from Install is not going to Uninstall the extension).
> 
> But what you mentioned about Uninstall not clearing the runOnce pref and
> thus not being able to reinstall the extension is definitely a bug, and we
> should fix it.

Felipe, can you please provide any kind of update on this issue?

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Attachment: Bug 1495749 - Allow add-ons to be updated via policy by uninstalling and reinstalling them. r=mkaply

Comment 8

[gabiganam]

(In reply to :Felipe Gomes (needinfo me!) from comment #7)
> Created attachment 9023435 [details]
> Bug 1495749 - Allow add-ons to be updated via policy by uninstalling and
> reinstalling them. r=mkaply

Thanks a lot Felipe!
I'd really love for this to be fixed in the earliest ESR version possible.
I'll stay tuned to this item for updates.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d96cbd9059cd
Allow add-ons to be updated via policy by uninstalling and reinstalling them. r=mkaply

Comment 10

[Natalia Csoregi [:nataliaCs]]

Backed out for browser_policy_extensions.js failures.

backout: https://hg.mozilla.org/integration/autoland/rev/b4e8f93bf4bbb14ba152d49ca3fb5538b2d599bd

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

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=210671422&repo=autoland&lineNumber=2488

14:45:09     INFO - TEST-PASS | browser/components/enterprisepolicies/tests/browser/browser_policy_extensions.js | Sanity check the temporary file doesn't exist. - true == true - 
14:45:09     INFO - Console message: [JavaScript Error: "this.mm.content is null; can't access its "devicePixelRatio" property" {file: "resource:///modules/FaviconLoader.jsm" line: 445}]
14:45:09     INFO - loadIcons@resource:///modules/FaviconLoader.jsm:445:9
14:45:09     INFO - FaviconLoader/this.iconTask<@resource:///modules/FaviconLoader.jsm:441:44
14:45:09     INFO - idleDispatch/</<@resource://gre/modules/PromiseUtils.jsm:40:21
14:45:09     INFO - 
14:45:09     INFO - Console message: 1541717108889	addons.xpi	WARN	Download of http://mochi.test:8888/browser/browser/components/enterprisepolicies/tests/browser/policytest_v0.2.xpi failed: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIZipReader.open]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/addons/XPIInstall.jsm :: XPIPackage :: line 327"  data: no] Stack trace: XPIPackage()@resource://gre/modules/addons/XPIInstall.jsm:327
14:45:09     INFO - get()@resource://gre/modules/addons/XPIInstall.jsm:227
14:45:09     INFO - loadManifest()@resource://gre/modules/addons/XPIInstall.jsm:1555
14:45:09     INFO - Console message: [JavaScript Error: "uncaught exception: undefined"]
14:45:09     INFO - Buffered messages finished
14:45:09     INFO - TEST-UNEXPECTED-FAIL | browser/components/enterprisepolicies/tests/browser/browser_policy_extensions.js | Uncaught exception - 
14:45:09     INFO - Not taking screenshot here: see the one that was previously logged
14:45:09     INFO - TEST-UNEXPECTED-FAIL | browser/components/enterprisepolicies/tests/browser/browser_policy_extensions.js | A promise chain failed to handle a rejection: (Unable to convert rejection reason to string.) - stack: onDownloadFailed@chrome://mochitests/content/browser/browser/components/enterprisepolicies/tests/browser/browser_policy_extensions.js:104:11
14:45:09     INFO - callInstallListeners@resource://gre/modules/AddonManager.jsm:1450:15
14:45:09     INFO - callInstallListeners@resource://gre/modules/AddonManager.jsm:2835:12
14:45:09     INFO - _callInstallListeners@resource://gre/modules/addons/XPIInstall.jsm:1953:12
14:45:09     INFO - downloadFailed@resource://gre/modules/addons/XPIInstall.jsm:2384:5
14:45:09     INFO - onStopRequest/<@resource://gre/modules/addons/XPIInstall.jsm:2357:11
14:45:09     INFO - Rejection date: Thu Nov 08 2018 14:45:08 GMT-0800 (Pacific Standard Time) - false == true - JS frame :: resource://testing-common/PromiseTestUtils.jsm :: assertNoUncaughtRejections :: line 257
14:45:09     INFO - Stack trace:
14:45:09     INFO - resource://testing-common/PromiseTestUtils.jsm:assertNoUncaughtRejections:257
14:45:09     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1123
14:45:09     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1089
14:45:09     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:987
14:45:09     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
14:45:09     INFO - Leaving test bound test_addon_reinstall
14:45:09     INFO - Entering test bound test_addon_uninstall
14:45:09     INFO - TEST-PASS | browser/components/enterprisepolicies/tests/browser/browser_policy_extensions.js | Sanity check the temporary file doesn't exist. - true == true - 
14:45:53     INFO - Not taking screenshot here: see the one that was previously logged
14:45:53     INFO - TEST-UNEXPECTED-FAIL | browser/components/enterprisepolicies/tests/browser/browser_policy_extensions.js | Test timed out - 
14:45:53     INFO - TEST-PASS | browser/components/enterprisepolicies/tests/browser/browser_policy_extensions.js | Engine is inactive at the end of the test -

Comment 11

[:Felipe Gomes (needinfo for replies!)]

The problem was due to phabsend not handling the binary file added (policytest_v0.2.xpi) correctly. I'll land this manually on inbound

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e5d78c219f5
Allow add-ons to be updated via policy by uninstalling and reinstalling them. r=mkaply

Comment 13

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/4e5d78c219f5

Comment 14

[gabiganam]

Hi Felipe thank you very much for this fix, is there any chance it can be pushed to version 64?

Thanks!

Comment 15

[:Felipe Gomes (needinfo for replies!)]

[Tracking Requested - why for this release]:

This is an improvement for the "Extensions" enterprise policy which allows admins to use policies to update an extension

Comment 16

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 9023435 [details]
Bug 1495749 - Allow add-ons to be updated via policy by uninstalling and reinstalling them. r=mkaply

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Enterprise Policies

User impact if declined: There's a policy called "Extensions" which can be used to install / uninstall extensions.  This improvement allow the same extension to be (first) uninstalled and reinstalled at the same time, allowing for a version upgrade (by reinstalling a newer version)

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Simple change to the code of this specific policy, covered by tests

String changes made/needed: none

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Enterprise Policy improvement

User impact if declined: We have been taking useful enterprise policies patches in ESR.  User who first requested this change has an ESR deployment

Fix Landed on Version: 65, asked for 64 uplift

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Simple change to the code of this specific policy, covered by tests

String or UUID changes made by this patch: none

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 9023435 [details]
Bug 1495749 - Allow add-ons to be updated via policy by uninstalling and reinstalling them. r=mkaply

improvement for enterprise policy, has test, looks safe enough; approved for 64.0b9 or 64.0b10

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Let's wait for QA verification before ESR uplift.

Comment 19

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce this issue using Firefox 64.0a1 (BuildId:20181002100236).

The Extensions.Uninstall action clears the Install runOnce pref and the extension is successfully reinstalled using Firefox 65.0a1 (BuildId:20181112220107) on Windows 10 64bit.

Leaving the qe-verify+ flag for beta verification.

Comment 20

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8c56b767fb96

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9023435 [details]
Bug 1495749 - Allow add-ons to be updated via policy by uninstalling and reinstalling them. r=mkaply

Policy change verified in nightly; let's uplift for ESR.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Needs a rebased patch for ESR60 uplift.

Comment 23

[Emil Ghitta, QA [:emilghitta]]

This is verified fixed using Firefox 64.0b10 (BuildId:20181115150739) on Windows 10 64bit.

Comment 24

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch rebased for ESR 60, r=mkaply a=lizzard

Rebased patch for ESR

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/15813976ae1e

Comment 26

[Emil Ghitta, QA [:emilghitta]]

This is verified fixed using Firefox 60.3.1esr (provided in comment 25) on Windows 10 64bit.

Comment 27

[gabiganam]

(In reply to Emil Ghitta, QA [:emilghitta] from comment #26)
> This is verified fixed using Firefox 60.3.1esr (provided in comment 25) on
> Windows 10 64bit.

Is there a scheduled release date for Firefox 60.3.1esr?

Comment 28

[Julien Cristau [:jcristau]]

This will be in 60.4.0esr which ships December 11.