Closed Bug 690934 Opened 10 years ago Closed 10 years ago
Uninstalling installed language pack reinstalls same language pack on restart
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
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.
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.
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.
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+
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.
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
Attachment #564634 - Flags: review?(mark.finkle)
Attachment #564634 - Flags: review?(mark.finkle) → review+
Backed out because the entire world turned orange https://hg.mozilla.org/integration/mozilla-inbound/rev/00676a1337d0
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.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111011 Firefox/10.0a1 Fennec/10.0a1 Samsung Galaxy SII (Android 2.3.4)
You need to log in before you can comment on or make changes to this bug.