The default bug view has changed. See this FAQ.

Uninstalling installed language pack reinstalls same language pack on restart

VERIFIED FIXED in Firefox 10

Status

Fennec Graveyard
General
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: aaronmt, Assigned: wesj)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 10
ARM
Android

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Using the locale picker, I uninstalled the recently installed locale, restarted and expected to see the fallback to en-US. Instead the same locale reinstalled.

STR:
1. Install en-US Nightly
2. Configure locale URL path *
3. Install any locale and restart
4. Uninstall the locale and restart

ER: Fall back to en-US as a default
AR: Reinstalls the recently uninstalled locale

Is this intended?

--
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20110930 Firefox/10.0a1 Fennec/10.0a1
Galaxy Tab 10.1 (Android 3.1)

* Using http://people.mozilla.org/~wjohnston/nightly_locales.xml
* Using en-US Nightly
(Reporter)

Updated

6 years ago
status-firefox10: --- → affected
(Assignee)

Comment 1

6 years ago
Doh! Not intentional. I made us smart so we don't do it on disable:

http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/extensions.js#429

but missed the uninstall case.
(Assignee)

Comment 2

6 years ago
Created attachment 563894 [details] [diff] [review]
WIP Patch

Hmm... I realized this is a bit tougher than I thought. What we really need to do is check if the language being uninstalled is the language you're using. But right now the AddonsManager doesn't give us much information about the locale you're uninstalling, except that it is a locale.

This hackily looks at addon.id and tries to yank out the locale name, which isn't guaranteed to be there. I'm going to ping Mossop and see if he knows a way around this?
Assignee: nobody → wjohnston
(In reply to Wesley Johnston (:wesj) from comment #2)
> Created attachment 563894 [details] [diff] [review] [diff] [details] [review]
> WIP Patch
> 
> Hmm... I realized this is a bit tougher than I thought. What we really need
> to do is check if the language being uninstalled is the language you're
> using. But right now the AddonsManager doesn't give us much information
> about the locale you're uninstalling, except that it is a locale.
> 
> This hackily looks at addon.id and tries to yank out the locale name, which
> isn't guaranteed to be there. I'm going to ping Mossop and see if he knows a
> way around this?

The AddonsManager doesn't know this, in fact by rights a locale pack could be for any number of locales, all that matters is what is in the chrome.manifest. The only thing I can think of right now is to use the chrome registry to map the url chrome://global/locale/global.dtd to its file uri and see if that is in the add-ons files.
(Reporter)

Updated

6 years ago
Flags: in-litmus?(aaron.train)
(Reporter)

Updated

6 years ago
Duplicate of this bug: 689678
(Assignee)

Comment 5

6 years ago
Created attachment 564415 [details] [diff] [review]
WIP Fix again

Another WIP, but this time much better. To reiterate what Mossop said, apparently the only way to get this information is to parse the chrome.manifest. There is no JS component sitting around to parse those that I know of. I also asked him today if he would want this code in AddonsManager and he said no.

So this adds a little helper to our extensionsView to do it for us.

Want to cleanup before I send in for review. Need to lazy load NetUtil.jsm and reduce some of the duplicate code in here. Write a little test.

Curious if you have any thoughts mfinkle? I'm only checking for lines like "locale browser en-US", which there can be multiple of in a single addon apparently. Since most of these are single locale packs though, you usually just see a lot of "locale mozapps ar", "locale alerts ar", "locale places ar", etc.
Attachment #563894 - Attachment is obsolete: true
Attachment #564415 - Flags: feedback?(mark.finkle)
Comment on attachment 564415 [details] [diff] [review]
WIP Fix again

Looks pretty good. You could just move the Cu.import into _getLocalesInAddon, since it's the only caller of NetUtil.
Attachment #564415 - Flags: feedback?(mark.finkle) → feedback+
(Reporter)

Comment 7

6 years ago
Created attachment 564586 [details]
Nightly (10/04) en-US - installing English

Side effect from this on 10/04 -- I get a dialog that Fennec is "Installing English"! (See screenshot). I didn't see this before. Did something related land?

STR: Install romania, restart, uninstall Romania.

See screenshot.
(Assignee)

Comment 8

6 years ago
Created attachment 564634 [details] [diff] [review]
Patch v1

Ready for review. This is pretty much the same with a little cleanup.

Also added a test on the _getLocalesInAddon function. Ideally we could actually install the addon, change languages, uninstall the addon, and see what happened. Unfortunately bootstrapped locales are not yet supported, no matter what you put in your install.rdf file.

I have that test written, and could mark as todo is if that is better, but this just tests the general function I wrote vs. a generic chrome.manifest with some valid and invalid entries.
Attachment #564415 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #564634 - Flags: review?(mark.finkle)
Attachment #564634 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 9

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/0a11abbdc4d9
Whiteboard: [inbound]
(Assignee)

Comment 10

6 years ago
Backed out because the entire world turned orange https://hg.mozilla.org/integration/mozilla-inbound/rev/00676a1337d0
(Assignee)

Comment 11

6 years ago
I pushed both patches separately to try yesterday and I'm 99% sure the other guy caused all the orange, but there seemed to be a lot of failures with this as well... I pushed it to try one more time to see if they were just randoms or not.
(Assignee)

Comment 12

6 years ago
pushed again:
http://hg.mozilla.org/integration/mozilla-inbound/rev/57c552ba5654
https://hg.mozilla.org/mozilla-central/rev/57c552ba5654
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
(Reporter)

Comment 14

6 years ago
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111011 Firefox/10.0a1 Fennec/10.0a1
Samsung Galaxy SII (Android 2.3.4)
Status: RESOLVED → VERIFIED
status-firefox10: affected → fixed
Whiteboard: [inbound]
(Reporter)

Updated

5 years ago
Flags: in-litmus?(aaron.train)
You need to log in before you can comment on or make changes to this bug.