Closed
Bug 690934
Opened 10 years ago
Closed 10 years ago
Uninstalling installed language pack reinstalls same language pack on restart
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox10 fixed)
VERIFIED
FIXED
Firefox 10
Tracking | Status | |
---|---|---|
firefox10 | --- | fixed |
People
(Reporter: aaronmt, Assigned: wesj)
References
Details
Attachments
(2 files, 2 obsolete files)
9.73 KB,
image/jpeg
|
Details | |
5.32 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
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•10 years ago
|
status-firefox10:
--- → affected
Assignee | ||
Comment 1•10 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•10 years ago
|
||
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
Comment 3•10 years ago
|
||
(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•10 years ago
|
Flags: in-litmus?(aaron.train)
Assignee | ||
Comment 5•10 years ago
|
||
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 6•10 years ago
|
||
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•10 years ago
|
||
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•10 years ago
|
||
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•10 years ago
|
Attachment #564634 -
Flags: review?(mark.finkle)
Updated•10 years ago
|
Attachment #564634 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 9•10 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/0a11abbdc4d9
Whiteboard: [inbound]
Assignee | ||
Comment 10•10 years ago
|
||
Backed out because the entire world turned orange https://hg.mozilla.org/integration/mozilla-inbound/rev/00676a1337d0
Assignee | ||
Comment 11•10 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•10 years ago
|
||
pushed again: http://hg.mozilla.org/integration/mozilla-inbound/rev/57c552ba5654
Comment 13•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/57c552ba5654
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
Reporter | ||
Comment 14•10 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)
Reporter | ||
Updated•9 years ago
|
Flags: in-litmus?(aaron.train)
You need to log in
before you can comment on or make changes to this bug.
Description
•