Last Comment Bug 818468 - Langpacks bundled in distribution/extensions are registered but disabled even if shown enabled
: Langpacks bundled in distribution/extensions are registered but disabled even...
Status: VERIFIED FIXED
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla21
Assigned To: Dave Townsend [:mossop]
:
:
Mentors:
: 806198 (view as bug list)
Depends on:
Blocks: 677092
  Show dependency treegraph
 
Reported: 2012-12-05 06:22 PST by Daniel Glazman (:glazou)
Modified: 2013-02-05 09:41 PST (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
disabled
+
disabled
+
disabled
verified


Attachments
patch rev 1 (7.72 KB, patch)
2013-01-08 18:12 PST, Dave Townsend [:mossop]
blair: review+
lukasblakk+bugs: approval‑mozilla‑aurora-
Details | Diff | Splinter Review
backout patch (11.44 KB, patch)
2013-01-15 12:03 PST, Dave Townsend [:mossop]
lukasblakk+bugs: approval‑mozilla‑aurora+
bajaj.bhavana: approval‑mozilla‑beta+
bajaj.bhavana: approval‑mozilla‑release+
Details | Diff | Splinter Review

Description Daniel Glazman (:glazou) 2012-12-05 06:22:45 PST
This seems to be a regression that started with restartless langpacks.
Before that change, langpacks added in XPI form into (appdir)/extensions
were registered, listed by the add-ons manager and enabled after restart.
After the change, they're registered, listed by the add-ons manager as
enabled but they are in fact disabled... Opening the add-ons manager, clicking
on "Disable" and then "Enable" enables the add-on.

I verified the change is the culprit forcing bootstrapping in my tree
turning the 'false' into a 'true' in
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/XPIProvider.jsm#766
Comment 1 Daniel Glazman (:glazou) 2012-12-05 06:26:30 PST
Correction, this is (appdir)/distribution/extensions. Sorry for that.
Comment 2 Axel Hecht [:Pike] 2012-12-05 06:47:13 PST
Do dictionaries work there?
Comment 3 Daniel Glazman (:glazou) 2012-12-05 07:00:44 PST
(In reply to Axel Hecht [:Pike] from comment #2)

> Do dictionaries work there?

I just tried. The behaviour is exactly the same.
Comment 4 Alex Keybl [:akeybl] 2013-01-07 13:11:45 PST
This bug (together with bug 677092) appears to have caused https://launchpad.net/bugs/1094376. We'll need to find an owner for FF19.
Comment 5 Wolfgang Rosenauer [:wolfiR] 2013-01-07 21:39:39 PST
Can someone please confirm if that is a dupe to bug 806198?
Comment 6 Dave Townsend [:mossop] 2013-01-07 21:49:45 PST
(In reply to Wolfgang Rosenauer [:wolfiR] from comment #5)
> Can someone please confirm if that is a dupe to bug 806198?

Maybe, probably no way to tell for sure unless you still have your profile files from before you got it working in bug 806198 or have a way to reproduce
Comment 7 Wolfgang Rosenauer [:wolfiR] 2013-01-08 07:01:30 PST
*** Bug 806198 has been marked as a duplicate of this bug. ***
Comment 8 Alex Keybl [:akeybl] 2013-01-08 16:04:04 PST
QA - can you confirm the steps to reproduce in bug 806198?
Comment 9 Dave Townsend [:mossop] 2013-01-08 16:08:09 PST
My testing is suggesting that this is worse than first thought and that language packs don't survive a restart right now. Can anyone else confirm/deny that?
Comment 10 Dave Townsend [:mossop] 2013-01-08 17:57:19 PST
(In reply to Dave Townsend (:Mossop) from comment #9)
> My testing is suggesting that this is worse than first thought and that
> language packs don't survive a restart right now. Can anyone else
> confirm/deny that?

Ok, I've verified that I was wrong here, some failsafe code from bug 619607 was stepping in and making sure that as long as langpacks were installed through the UI they work correctly.
Comment 11 Dave Townsend [:mossop] 2013-01-08 18:12:04 PST
Created attachment 699533 [details] [diff] [review]
patch rev 1

It's important to always call loadBootstrapScope for a restartless add-on as that takes care of registering it in bootstrappedAddons which is what we use to load these add-ons on startup. Bug 677092 skipped that for langpacks though it wasn't obvious because the code in 619607 was forcing the add-on into bootstrappedAddons after a restart but only if the langpack was installed through the UI. Installing another add-on later probably causes Firefox to pick up the faulty langpacks too.

Note I'm also setting bootstrapScope for the add-on just to stop us calling back into loadBootstrapScope later, it doesn't matter much what we set it to.

Added tests verifying that langpacks are properly unloaded on the first shutdown (they weren't before) and that those detected in the profile or distribution direction (the same really but lets be thorough) get started correctly and survive a restart.
Comment 12 Dave Townsend [:mossop] 2013-01-08 18:13:33 PST
Also spinning this through try but I don't expect there to be any issues.
Comment 13 juan becerra [:juanb] 2013-01-09 11:17:23 PST
Does anyone have an example from AMO I use to reproduce this problem and see if it is happening in Win/Mac? If do this from AMO will I not be able to reproduce the problem (according to comment #10)?
Comment 14 Dave Townsend [:mossop] 2013-01-09 11:22:53 PST
(In reply to juan becerra [:juanb] from comment #13)
> Does anyone have an example from AMO I use to reproduce this problem and see
> if it is happening in Win/Mac? If do this from AMO will I not be able to
> reproduce the problem (according to comment #10)?

The way to reproduce this is to take a langpack xpi, drop it into <profile>/extensions/<id>.xpi and then start Firefox. It should show in the add-ons manager as enabled but locale switcher etc. won't list it.
Comment 15 juan becerra [:juanb] 2013-01-09 14:31:46 PST
I've been trying to reproduce this with several language packs using the instructions in comment #14, but when I start the browser I'm prompted to "modify Firefox with the following add-on" and in about:addons the language pack appears disabled.
Comment 16 Dave Townsend [:mossop] 2013-01-09 14:35:42 PST
(In reply to juan becerra [:juanb] from comment #15)
> I've been trying to reproduce this with several language packs using the
> instructions in comment #14, but when I start the browser I'm prompted to
> "modify Firefox with the following add-on" and in about:addons the language
> pack appears disabled.

Bah, forgot about that sorry. Ok instead put them into <appdir>/distribution/extensions/<id>.xpi and then delete your profile and start Firefox.
Comment 17 juan becerra [:juanb] 2013-01-09 15:18:38 PST
After that last suggestion I see the language pack in about:addons, and it looks enabled, but when I install the Locale Switcher addon and restart to see if the language pack is enabled, the language pack is listed and it works if I select it and restart. I tried doing something similar with a dictionary, but that never got listed in about:addons.

I am not sure how easy it would be for an average user to get into this situation.
Comment 18 bhavana bajaj [:bajaj] 2013-01-09 16:18:07 PST
Considering QA feedback regarding the uncertainty of this user scenario being uncommon and given the lack of early feedback on support, we are planning to unthrottle tomorrow as scheduled. 

We will react accordingly if the landscape changes based on user feedback in the coming few days.
Comment 19 Wolfgang Rosenauer [:wolfiR] 2013-01-09 21:42:10 PST
Just to explain my usecase where all of my openSUSE users are affected:
We are shipping language packs (self created from the repos along with Firefox) in $(appdir)/extensions (not xpi but flat files what shouldn't make a difference). In addition we have intl.locale.matchOS set to true so Firefox comes up with the language the user uses in his environment automatically. With 18.0 this is currently broken until the users disables/enables the language in the addon manager. Then it works until the next update.
Comment 20 Jan Steffens 2013-01-09 23:37:08 PST
Same here. All users of Firefox langpacks on Arch Linux are affected as well. We ship the xpi files in (appdir)/extensions.
Comment 21 Mike Hommey [:glandium] 2013-01-10 00:20:43 PST
Same on Debian.
Comment 22 Dave Townsend [:mossop] 2013-01-10 09:38:38 PST
To be clear, we're planning to have the fix for this in Firefox 19 and the recommendation for Linux distros that are affected by this is to back out bug 677092 from their Firefox 18 builds.
Comment 23 Alex Keybl [:akeybl] 2013-01-13 13:07:14 PST
If we do spin an 18.0.1, we'll back out bug 677092.
Comment 24 Alex Keybl [:akeybl] 2013-01-13 13:07:50 PST
That may even be the best idea for FF19, if this patch carries any risk.
Comment 25 Paul Silaghi, QA [:pauly] 2013-01-14 07:52:36 PST
(In reply to Dave Townsend (:Mossop) from comment #14)
> The way to reproduce this is to take a langpack xpi, drop it into
> <profile>/extensions/<id>.xpi and then start Firefox. It should show in the
> add-ons manager as enabled but locale switcher etc. won't list it.

(In reply to Dave Townsend (:Mossop) from comment #16)
> Bah, forgot about that sorry. Ok instead put them into
> <appdir>/distribution/extensions/<id>.xpi and then delete your profile and
> start Firefox.

I don't see the language pack in addons manager after copying to <appdir>/distribution/extensions/<id>.xpi, both on FF 17 and 18. What am I doing wrong? Should I enabled something in about:config ?
Comment 26 Ed Morley [:emorley] 2013-01-14 09:09:13 PST
https://hg.mozilla.org/mozilla-central/rev/c5c4cff29edd
Comment 27 Dave Townsend [:mossop] 2013-01-14 09:23:31 PST
(In reply to Paul Silaghi [QA] from comment #25)
> (In reply to Dave Townsend (:Mossop) from comment #14)
> > The way to reproduce this is to take a langpack xpi, drop it into
> > <profile>/extensions/<id>.xpi and then start Firefox. It should show in the
> > add-ons manager as enabled but locale switcher etc. won't list it.
> 
> (In reply to Dave Townsend (:Mossop) from comment #16)
> > Bah, forgot about that sorry. Ok instead put them into
> > <appdir>/distribution/extensions/<id>.xpi and then delete your profile and
> > start Firefox.
> 
> I don't see the language pack in addons manager after copying to
> <appdir>/distribution/extensions/<id>.xpi, both on FF 17 and 18. What am I
> doing wrong? Should I enabled something in about:config ?

Did you delete your profile before starting Firefox?
Comment 28 Paul Silaghi, QA [:pauly] 2013-01-14 14:18:27 PST
Yes, I removed all the profiles in %appdata%.

Why there isn't by default an "<appdir>/distribution/" folder in my FF 18 release dir, but there is one in the FF 18b6 dir ?
Comment 29 Dave Townsend [:mossop] 2013-01-14 14:31:48 PST
(In reply to Paul Silaghi [QA] from comment #28)
> Yes, I removed all the profiles in %appdata%.
> 
> Why there isn't by default an "<appdir>/distribution/" folder in my FF 18
> release dir, but there is one in the FF 18b6 dir ?

We ship testpilot with beta and aurora builds but not release builds.
Comment 30 bhavana bajaj [:bajaj] 2013-01-14 14:44:05 PST
Dave, comment# 23 seems to be a safer approach to get this resolved for a possible Fx18.0.1 in terms of avoiding any surprising regressions which may come up if we consider the fwd fix given the limited testing we have had .

Can you please help prepare a backout patch in preparation and nominate for for approval-mozilla-release ?
Comment 31 Dave Townsend [:mossop] 2013-01-14 17:22:58 PST
(In reply to bhavana bajaj [:bajaj] from comment #30)
> Dave, comment# 23 seems to be a safer approach to get this resolved for a
> possible Fx18.0.1 in terms of avoiding any surprising regressions which may
> come up if we consider the fwd fix given the limited testing we have had .
> 
> Can you please help prepare a backout patch in preparation and nominate for
> for approval-mozilla-release ?

Running through try now: https://tbpl.mozilla.org/?tree=Try&rev=b595c1c3589f
Comment 32 bhavana bajaj [:bajaj] 2013-01-15 11:19:26 PST
(In reply to Dave Townsend (:Mossop) from comment #31)
> (In reply to bhavana bajaj [:bajaj] from comment #30)
> > Dave, comment# 23 seems to be a safer approach to get this resolved for a
> > possible Fx18.0.1 in terms of avoiding any surprising regressions which may
> > come up if we consider the fwd fix given the limited testing we have had .
> > 
> > Can you please help prepare a backout patch in preparation and nominate for
> > for approval-mozilla-release ?
> 
> Running through try now: https://tbpl.mozilla.org/?tree=Try&rev=b595c1c3589f

Dave, can we please have the patch landed & nominated for approval-mozilla-release and approval-mozilla-beta as per comment# 24 ?
Comment 33 Dave Townsend [:mossop] 2013-01-15 12:03:36 PST
Created attachment 702450 [details] [diff] [review]
backout patch

Straight backout, applied cleanly.

[Approval Request Comment]
Regression caused by (bug #): 818468
User impact if declined: Broken langpacks on linux distros
Testing completed (on m-c, etc.): It's a backout so this was the status quo for Fx4-17
Risk to taking this patch (and alternatives if risky): None
Comment 34 Dave Townsend [:mossop] 2013-01-15 12:07:56 PST
Comment on attachment 699533 [details] [diff] [review]
patch rev 1

I think we should take the real fix on aurora once it's been through a few more days on nightly. It's likely safe enough for beta too but I'm in no rush to push it there unless someone else thinks it is urgent.
Comment 35 bhavana bajaj [:bajaj] 2013-01-15 13:20:06 PST
Comment on attachment 702450 [details] [diff] [review]
backout patch

Please land as soon as possible as we are going to build with FF19 beta & 18.0.1 very soon . Thanks !
Comment 37 Lukas Blakk [:lsblakk] use ?needinfo 2013-01-16 12:32:49 PST
Comment on attachment 699533 [details] [diff] [review]
patch rev 1

Given the backouts on beta/release I think this should ride the trains, seeing it's not a regression. I'll approve the backout patch for Aurora as well.
Comment 38 Daniel Glazman (:glazou) 2013-01-17 05:48:28 PST
Just a big thank you for fixing this, the issue was a nightmare for me
working on BlueGriffon!
Comment 39 Paul Silaghi, QA [:pauly] 2013-01-17 07:38:31 PST
(In reply to juan becerra [:juanb] from comment #17)
> After that last suggestion I see the language pack in about:addons, and it
> looks enabled, but when I install the Locale Switcher addon and restart to
> see if the language pack is enabled, the language pack is listed and it
> works if I select it and restart.

I tested this with FF en-US, german language pack xpi, Ubuntu 12.04.
I confirm the same behavior on FF 18, 18.0.1 and 19b2. I think the initial problem couldn't be reproduced using the STR in comment 14, 16 because the Locale Switcher, Quick Locale Switcher addons require browser restart.
Anyway, I think this bug can be marked as verified considering reporter's comment 38.
Comment 40 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-17 09:56:16 PST
Daniel, can you confirm this bug no longer reproduces for you in both Firefox 18.0.1 and Firefox 19.0b2?
Comment 41 Ryan VanderMeulen [:RyanVM] 2013-01-19 21:18:27 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/08c0832fbb6e

(I think disabled is more accurate since this was backed out on the branches)
Comment 42 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-02-05 09:41:33 PST
Marking verified fixed based on comment 38 and 39. Please reopen if you can reproduce this bug in a current build.

Note You need to log in before you can comment on or make changes to this bug.