Closed Bug 1237820 Opened 4 years ago Closed 4 years ago

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

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla47
Tracking Status
firefox43 + wontfix
firefox44 - wontfix
firefox45 + verified
firefox46 + verified
firefox47 --- verified
firefox-esr38 - wontfix

People

(Reporter: mossop, Assigned: mossop)

References

Details

Attachments

(3 files)

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.
[Tracking Requested - why for this release]:
This is an issue affecting developers sideloading add-ons into Firefox.
So, any estimated release date or version regarding this fix? Currently our add-on is on latest FF (43.0.4)
(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.
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?
Flags: needinfo?(kev)
Flags: needinfo?(amckay)
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.
Flags: needinfo?(rkothari)
(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.
Flags: needinfo?(kev)
Flags: needinfo?(amckay)
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.
Flags: needinfo?(rkothari) → needinfo?(dtownsend)
Kev, asking you the same Qs as mentioned in comment 7.
Flags: needinfo?(kev)
[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.
Flags: needinfo?(dtownsend)
Duplicate of this bug: 1241044
Blocks: 1240191
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/
Attachment #8711813 - Flags: review?(rhelmer)
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.
Flags: qe-verify?
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)` ?
Attachment #8711813 - Flags: review?(rhelmer) → review+
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 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/
Attachment #8711813 - Attachment description: MozReview Request: Bug 1237820: Track whether a user has been offered a sideloaded add-on or not. r?rhelmer → MozReview Request: Bug 1237820: Track whether a user has been offered a sideloaded add-on or not. 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/
Attachment #8712399 - Flags: review?(rhelmer)
Comment on attachment 8712399 [details]
MozReview Request: Bug 1237820: Fix the tests. r?rhelmer

https://reviewboard.mozilla.org/r/32531/#review29271
Attachment #8712399 - Flags: review?(rhelmer) → review+
https://hg.mozilla.org/mozilla-central/rev/c8f9b2cd1688
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Dave, should we uplift this to Aurora46 and Beta45?
Flags: needinfo?(dtownsend)
(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.
Flags: needinfo?(dtownsend)
Andrei can someone from your team verify the fix before we uplift? Thanks!
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(andrei.vaida)
(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.
Flags: needinfo?(andrei.vaida)
QA Contact: vasilica.mihasca
(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?
Flags: needinfo?(dtownsend)
(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.
Flags: needinfo?(dtownsend)
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.
Flags: needinfo?(dtownsend)
(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.
Flags: needinfo?(dtownsend)
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
Attachment #8717133 - Flags: approval-mozilla-beta?
Attachment #8717133 - Flags: approval-mozilla-aurora?
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.
Attachment #8717133 - Flags: approval-mozilla-beta?
Attachment #8717133 - Flags: approval-mozilla-beta+
Attachment #8717133 - Flags: approval-mozilla-aurora?
Attachment #8717133 - Flags: approval-mozilla-aurora+
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.
Flags: needinfo?(dtownsend)
(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.
Flags: needinfo?(dtownsend)
Depends on: 1249074
(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.
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(kev)
You need to log in before you can comment on or make changes to this bug.