1237820 - Firefox doesn't display the sideload UI when a sideloaded add-on that was previously appDisabled can now be enabled

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

When a sideloaded add-on is first installed we normally display a UI to allow the user to enable it. In the case where the add-on can't be loaded however (compatibility, signed state, etc.) we don't display the UI.

If the add-on is later updated we still don't show it because by then the add-on has already been installed and we simply assume the user actively opted out of using it.

This has been a bug since we added the sideloading UI in Firefox but has become more obvious now so many add-ons are appDisabled on install because they haven't yet been signed.

Comment 1

[Dave Townsend [:mossop]]

[Tracking Requested - why for this release]:
This is an issue affecting developers sideloading add-ons into Firefox.

Comment 2

[Sandipan]

So, any estimated release date or version regarding this fix? Currently our add-on is on latest FF (43.0.4)

Comment 3

[Dave Townsend [:mossop]]

(In reply to Sandipan from comment #2)
> So, any estimated release date or version regarding this fix? Currently our
> add-on is on latest FF (43.0.4)

I'm hoping to have a fix that can at least go out in Firefox 44 but it may not fix the problem for add-ons currently stuck in this state.

Comment 4

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

I think we will have to say wontfix for 43. Firefox 44 is coming up pretty fast (Jan. 26 or so.)

Andy and Kev, is this the same issue that you all brought up late in December?

Comment 5

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

Tracking since this appears to affect many addon developers. Ritu just a heads up for you for Beta as well as it may end up being a late uplift request.

Comment 6

[Andy McKay]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #4)
> I think we will have to say wontfix for 43. Firefox 44 is coming up pretty
> fast (Jan. 26 or so.)
> 
> Andy and Kev, is this the same issue that you all brought up late in
> December?

It's actually a different one.

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Dave, I will gtb the last beta of this cycle tomorrow. Do we have a fix ready? Is the workaround in this case to change the addon signing pref to off? Would that help? Or does this not have any workaround? How many addons are going to be impacted here? This will help me determine whether this is a release blocker or not.

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Kev, asking you the same Qs as mentioned in comment 7.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

[Tracking Requested - why for this release]:

Given that we don't have a fix in the works and we are into RC mode, it's too late and this is now a wontfix for Fx44.

Comment 11

[Dave Townsend [:mossop]]

Attachment: MozReview Request: Bug 1237820: Track whether a user has been offered a sideloaded add-on or not. r=rhelmer

Previously we just checked every newly sideloaded add-on to decide whether to
offer it to the user for opt-in. This adds a new "seen" property (naming could
be better if you have other suggestions) which tracks whether we've ever shown
the opt-in UI for the add-on. It defaults to true for all add-ons and is only
set to false for sideloaded add-ons that default to being disabled on install.
The seen flag can be set to true through the API but cannot be reset to false
as that would allow add-ons to forcibly re-present themselves to the user when
disabled.

The opt-in UI sets the seen flag to true only when it has focus which fixes a
long-standing bug where if you accept the first add-on you see and restart the
other tabs might not show up.

The one slight downside of this approach is that it now requires loading the
full add-ons database on every startup in order to check the seen flag for all
installed add-ons. There are hacky ways we might get around this but they all
involve overloading prefs with even more object data. The good thing is that
we do the load and check asynchronously after most of startup is complete and
the UI is fully loaded so there shouldn't be any percieved impact to startup
time. I've run multiple talos runs to verify that none of the numbers appear to
regress.

Review commit: https://reviewboard.mozilla.org/r/32319/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32319/

Comment 12

[Dave Townsend [:mossop]]

We don't have any automated tests that cover the whole flow of detecting a sideloaded add-on and presenting the UI on startup. None of the suites support this use case. So we could do with some manual QA verification here. Some steps like this:

1. Sideload an add-on that should work.
2. Start Firefox.
3. A tab should open offering the add-on to the user.
4. Restart Firefox.
5. The tab should no longer be present.

1. Sideload an add-on that can't work (incompatible or unsigned).
2. Start Firefox.
3. No tab should be presented.
4. Exit Firefox and replace the sideloaded add-on with one that works.
5. Start Firefox.
6. A tab opens offering the add-on to the user.

Comment 13

[Robert Helmer [:rhelmer]]

Comment on attachment 8711813 [details]
MozReview Request: Bug 1237820: Track whether a user has been offered a sideloaded add-on or not. r=rhelmer

https://reviewboard.mozilla.org/r/32319/#review29053

::: browser/components/nsBrowserGlue.js:1149
(Diff revision 1)
> -            return;
> +        if (addon.seen !== false) {

Why not just `(!addon.seen)` ?

Comment 14

[Dave Townsend [:mossop]]

https://reviewboard.mozilla.org/r/32319/#review29053

> Why not just `(!addon.seen)` ?

The delight of multiple add-on providers is that different addon objects have differing properties defined. So we have to check both that the property is defined and that it has the value we want. If addon.seen is undefined then !addon.seen === true

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f79aecb41a1e

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/19eb736ea701

Comment 17

[Dave Townsend [:mossop]]

Comment on attachment 8711813 [details]
MozReview Request: Bug 1237820: Track whether a user has been offered a sideloaded add-on or not. r=rhelmer

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32319/diff/1-2/

Comment 18

[Dave Townsend [:mossop]]

Attachment: MozReview Request: Bug 1237820: Fix the tests. r?rhelmer

I changed the seen property to only be false when we auto-disable sideloaded
add-ons and hadn't updated the tests to match. This in turn meant we got hit
with some issues with the tests seeing changes to sideloaded add-ons, mainly
down to bug 1231849 so there are some date fixes here.

Review commit: https://reviewboard.mozilla.org/r/32531/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32531/

Comment 19

[Robert Helmer [:rhelmer]]

Comment on attachment 8712399 [details]
MozReview Request: Bug 1237820: Fix the tests. r?rhelmer

https://reviewboard.mozilla.org/r/32531/#review29271

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/c8f9b2cd1688

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c8f9b2cd1688

Comment 22

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Dave, should we uplift this to Aurora46 and Beta45?

Comment 23

[Dave Townsend [:mossop]]

(In reply to Ritu Kothari (:ritu) from comment #22)
> Dave, should we uplift this to Aurora46 and Beta45?

Yes but I'd like to see some manual QA from comment 12 done first ideally.

Comment 24

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

Andrei can someone from your team verify the fix before we uplift? Thanks!

Comment 25

[Andrei Vaida [:avaida]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #24)
> Andrei can someone from your team verify the fix before we uplift? Thanks!

Assigning Vasilica here for verification.

Comment 26

[Vasilica Tamas, QA [:vtamas]]

(In reply to Dave Townsend [:mossop] from comment #12)
> We don't have any automated tests that cover the whole flow of detecting a
> sideloaded add-on and presenting the UI on startup. None of the suites
> support this use case. So we could do with some manual QA verification here.
> Some steps like this:
> 
> 1. Sideload an add-on that should work.
> 2. Start Firefox.
> 3. A tab should open offering the add-on to the user.
> 4. Restart Firefox.
> 5. The tab should no longer be present.
> 
> 1. Sideload an add-on that can't work (incompatible or unsigned).
> 2. Start Firefox.
> 3. No tab should be presented.
> 4. Exit Firefox and replace the sideloaded add-on with one that works.
> 5. Start Firefox.
> 6. A tab opens offering the add-on to the user.

Dave If you need manual testing for any add-on related bug, please assign me a needinfo (or as qa contact) and I will gladly help you.

I’ve tested this bug following the steps mentioned in Comment 12, but no tab is displayed after unsigned/incompatible add-on is replaced with a signed one and it still appears as disabled in Addons Manager: http://i.imgur.com/oht5jCB.jpg.


Tested on Firefox 47.0a1 (2016-01-28) under Windows 10 64-bit and Ubuntu 14.04 32-bit.

Any thoughts about this?

Comment 27

[Dave Townsend [:mossop]]

(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #26)
> (In reply to Dave Townsend [:mossop] from comment #12)
> > We don't have any automated tests that cover the whole flow of detecting a
> > sideloaded add-on and presenting the UI on startup. None of the suites
> > support this use case. So we could do with some manual QA verification here.
> > Some steps like this:
> > 
> > 1. Sideload an add-on that should work.
> > 2. Start Firefox.
> > 3. A tab should open offering the add-on to the user.
> > 4. Restart Firefox.
> > 5. The tab should no longer be present.
> > 
> > 1. Sideload an add-on that can't work (incompatible or unsigned).
> > 2. Start Firefox.
> > 3. No tab should be presented.
> > 4. Exit Firefox and replace the sideloaded add-on with one that works.
> > 5. Start Firefox.
> > 6. A tab opens offering the add-on to the user.
> 
> Dave If you need manual testing for any add-on related bug, please assign me
> a needinfo (or as qa contact) and I will gladly help you.

Ok thanks, I never quite know what the way to do these things are.

> I’ve tested this bug following the steps mentioned in Comment 12, but no tab
> is displayed after unsigned/incompatible add-on is replaced with a signed
> one and it still appears as disabled in Addons Manager:
> http://i.imgur.com/oht5jCB.jpg.

You might be hitting bug 1231849. If the add-on is sideloaded as a directory then make sure that the last modified time of the new install.rdf file is very new. A simple "touch install.rdf" on linux or OSX will do that. If it's installed as a packed XPI file then do the same to the XPI. Let me know if that solves it.

Comment 28

[Vasilica Tamas, QA [:vtamas]]

Tested using different methods to install a sideloaded add-on (both directory and packed XPI) and these is what I have noticed:

Method I :

1.Create a Firefox profile (Profile A) and close it.
2.Sideload an unsigned/incompatible addon.
3.Start Profile A -> No tab is presented.
4.Close profile A.
5.Create a second Firefox profile (Profile B) and install from addons.mozilla.org the same add-on.
6.Copy the profile B extension folder.
7.Replace the profile A extension folder with profile B extension folder.
8.Start Profile A -> A tab opens offering the add-on to the user

- Following this method both directory and packed XPI add-on are successfully installed.


Method II :

1.Create a folder that contains the unzipped unsigned/corrupted add-on.
2.Create a second folder that contains the unzipped signed add-on. 
- See screenshot: http://i.imgur.com/nozycQM.jpg
- Note that the install.rdf modified time of the signed add-on has a more recent date than the unsigned add-on
3.Copy & Paste in the Firefox extensions directory the unsigned/corrupted folder.
4.Start Firefox -> No tab is presented.
5.Replace the unsigned/corrupted add-on folder with the signed one.
6.Start Firefox ->No tab is presented.

- A packed XPI file is successfully installed following Method II.
- This method fails for an add-on sideloaded as directory. Why does this happen? because the install.rdf file of signed add-on might not have the current time? 



Tested across all platforms: Windows 10 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5.

Comment 29

[Dave Townsend [:mossop]]

(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #28)
> Tested using different methods to install a sideloaded add-on (both
> directory and packed XPI) and these is what I have noticed:
> 
> Method I :
> 
> 1.Create a Firefox profile (Profile A) and close it.
> 2.Sideload an unsigned/incompatible addon.
> 3.Start Profile A -> No tab is presented.
> 4.Close profile A.
> 5.Create a second Firefox profile (Profile B) and install from
> addons.mozilla.org the same add-on.
> 6.Copy the profile B extension folder.
> 7.Replace the profile A extension folder with profile B extension folder.
> 8.Start Profile A -> A tab opens offering the add-on to the user
> 
> - Following this method both directory and packed XPI add-on are
> successfully installed.
> 
> 
> Method II :
> 
> 1.Create a folder that contains the unzipped unsigned/corrupted add-on.
> 2.Create a second folder that contains the unzipped signed add-on. 
> - See screenshot: http://i.imgur.com/nozycQM.jpg
> - Note that the install.rdf modified time of the signed add-on has a more
> recent date than the unsigned add-on
> 3.Copy & Paste in the Firefox extensions directory the unsigned/corrupted
> folder.
> 4.Start Firefox -> No tab is presented.
> 5.Replace the unsigned/corrupted add-on folder with the signed one.
> 6.Start Firefox ->No tab is presented.
> 
> - A packed XPI file is successfully installed following Method II.
> - This method fails for an add-on sideloaded as directory. Why does this
> happen? because the install.rdf file of signed add-on might not have the
> current time? 

Possibly yes. The mechanism for detecting the change are a bit complicated when an add-on is disabled but the short story is that the new install.rdf has to have a modification time newer than any other file in the add-on's directory, including the directory itself. So if you created the folder in step 1 later than the time of the new install.rdf (15:08) then Firefox won't see the change.

Comment 30

[Dave Townsend [:mossop]]

Attachment: aurora/beta patch

Approval Request Comment
[Feature/regressing bug #]: None
[User impact if declined]: Users may not be offered sideloaded add-ons when they become available
[Describe test coverage new/current, TreeHerder]: Automated tests and manual QA
[Risks and why]: Low risk, should only impact the sideloading feature and we have good tests suggesting it doesn't
[String/UUID change made/needed]: None

Comment 31

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8717133 [details] [diff] [review]
aurora/beta patch

Has a lot of tests, improve the side loaded addons situation, taking it.
Should be in 45 beta 5.

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ef9aeb72b407

Comment 33

[Carsten Book [:Tomcat]]

(In reply to Carsten Book [:Tomcat] from comment #32)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/ef9aeb72b407

make that https://hg.mozilla.org/releases/mozilla-aurora/rev/05675ccd0eeb

Comment 34

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2427b0bca815

Comment 35

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Dave, do you think we should uplift this to esr38? I am reviewing possible fixes to take in esr38.7 and this one seems to fit the bill from a severity point of view.

Comment 36

[Dave Townsend [:mossop]]

(In reply to Ritu Kothari (:ritu) from comment #35)
> Dave, do you think we should uplift this to esr38? I am reviewing possible
> fixes to take in esr38.7 and this one seems to fit the bill from a severity
> point of view.

I think really the severity is pretty low here. Although technically every ESR since we implemented the sideloading restrictions has been affected we've never heard of anyone complain about this bug until signing was required, and signing isn't required in 38. So it affects 38 in small numbers.

45 is the next ESR isn't it? I'd be inclined to just let it ride with that unless we think there is enough impact for me to backport this. I'd need to verify the fix applies and that it does the right things on the older branch. It's probably straightforward enough.

Comment 37

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Dave Townsend [:mossop] from comment #36)
> (In reply to Ritu Kothari (:ritu) from comment #35)
> > Dave, do you think we should uplift this to esr38? I am reviewing possible
> > fixes to take in esr38.7 and this one seems to fit the bill from a severity
> > point of view.
> 
> I think really the severity is pretty low here. Although technically every
> ESR since we implemented the sideloading restrictions has been affected
> we've never heard of anyone complain about this bug until signing was
> required, and signing isn't required in 38. So it affects 38 in small
> numbers.
> 
> 45 is the next ESR isn't it? I'd be inclined to just let it ride with that
> unless we think there is enough impact for me to backport this. I'd need to
> verify the fix applies and that it does the right things on the older
> branch. It's probably straightforward enough.

Thanks Dave! Wontfix for esr38 given that nobody has complained about this issue so far and it will be fixed in ESR45.

Comment 38

[Vasilica Tamas, QA [:vtamas]]

I was able to reproduce this issue on Firefox 44.0.2 under Windows 10 64-bit.

Verified fixed on Firefox 47.0a1 (2016-02-24/25), Firefox 46.0a2 (2016-02-24/25) and Firefox 45 beta 9 (20160223142613) under Windows 10 64-bit, Ubuntu 12.04 32-bit and Mac OS X 10.10.4.