Currently, there is no UI in the product, including ProfileManager, Prefs dlg, and View menu to un-install the obsolete packs.

The UI was getting pretty cramped so the uninstall buttons are over on the left. Otherwise the list boxes get very small and the dialog gets just plain ugly.

On the first patch I forgot that I made it so that the user couldn't accidentally select an obsolete pack. This prevents them from selecting a pack to uninstall. Catch-22. Tao and I believe that since there's a "(needs update)" message next to obsolete packs it would be best to allow the user to select that pack for uninstallation.

Comment on attachment 87288 [details] [diff] [review] Removed the code that prevents obsolete pack selection r=tao

There may someday be a XPInstall package uninstaller that will do this.

Comment on attachment 87288 [details] [diff] [review] Removed the code that prevents obsolete pack selection Has the UI change gotten mcarlson and adt approval? Has making this cramped dialog more cramped gotten UE approval? Seems late for this kind of change w/out checking with a lot of folks. Please post a screen shot of the updated dialog and check w/Lori's team (and probably Trudelle's team who own the prefs dialogs as a whole). >+ function UninstallContentPack() >+ { >+ var contentList = document.getElementById("contentPackList"); >+ var selectedContentItem = contentList.selectedItems[0]; >+ var contentPackName = selectedContentItem.getAttribute("value"); >+ dump("The content name is: " + contentPackName + "\n"); >+ var chromeRegistry = Components.classes["@mozilla.org/chrome/chrome-registry;1"].getService(Components.interfaces.nsIXULChromeRegistry); >+ chromeRegistry.uninstallLocale(contentPackName, false); >+ contentList.selectedIndex = 0; >+ } Why do you assume global chrome? The language pack install scripts will put them in the profile if they can't write to the global location (fairly common on Unix). I don't know how you could tell if a locale was global or profile ahead of time, but you should at least check for errors from the call to uninstall. Maybe you could try the other one after that? When does the item get removed from the list? Does the uninstall call itself force a UI refresh on the data source or something? What happens if you uninstall the currently selected locale? What happens if you uninstall the last locale? Excuse my paranoia, but I remember there being some reason we don't allow users to uninstall the global skins, and skins are much easier to deal with in the chrome system than locales. >+ function UninstallLanguagePack() Same worries. >+ dump("The language name is: " + languageName + "\n"); Only leave dumps if you are fully prepared for a visit from nazis^H^H^H^H^Hrepresentatives from the Dump-dump() campaign.

Good review Dan. I was trying to get this done before I leave for my new group but now I'm thinking that it needs to be thought out more. You raised some good points. (Your paranoia is well founded.) In the current patch (87288) I had forgotten that locales can be installed into the profile. My system obviously doesn't have that situation so I set the second parameter to "true" for global. The item is removed from the list immediately. I didn't test removal of the currently selected locale or the last one. Again, haste makes waste. Let me talk to Tao about what he wants to do with this. It seems like there should be a bunch of players involved (UE, xpfe, etc.)

new owner ->tao

Tao, I might be a good owner for this, tentatively reassigning.

screenshot capturing the proposed modifications to the preference panel

Albeit this patch should be good enough for 1.2a, we might want to clean up and optimize the panel for 1.2b. I have some ideas for that and hope to be able to return to it. Dan, could you please comment on this?

nhotta/cmanske/timeless - I just got UI sign-off from Marlon. Would you have a few minutes for a quick review? This enhancement one of the top i18n/l0n requests. I also own bug 161508 now and will try to complete it in the 1.2 time frame, but that's up in the air since dprice left. I'll file a follow-up bug to move more frequently used service references to nsPrefwindow and weed out some of the unnecessary code from individual prefpanels.

I just looked through the latest patch, and AFAICT it looks good, nice work! What came to my mind when reading: 1) Is there a bug about implementing uninstall for globally installed themes? We currently can only uninstall profile-installed themes and if it works for locale, it should work for themes as well... 2) Wouldn't it be better to also do a reordering of that panel so that laguage packs are above content packs? Should I open a new bug for that? BTW, shouldn't you obsolete the attachment named "yet another patch iteration"?

Comment on attachment 96026 [details] [diff] [review] yet another patch iteration obsoleting

I've been pinging a number of people for a review. Timeless sent some private comments, which have been largely incorporated into the patch. Clearly, some progress on the reviewing front needs to occur, if we want to have this in 1.2. Anyone up for a r=?

I've looked it over and don't see any problems offhand, but my build is currently horked and I plan to update everything tomorrow. As soon as I have bits to test tomorrow I'll be happy to provide the r=.

Charlie, that would be great :-)

Charlie, I've just addressed some bitrot issues - the patch applies cleanly to and works well with a current trunk build... We really need to get this on the 1.2 train ASAP.

looks like I might be able to get this r= later today :-) cc'ing alecf, hopefully he might have some time to sr= this later this week.

Comment on attachment 100011 [details] [diff] [review] sigh, this has been bit rotting for too long now - synching up with a current trunk pull please verify your patch on Linux and Mac.

Comment on attachment 100011 [details] [diff] [review] sigh, this has been bit rotting for too long now - synching up with a current trunk pull sr=alecf

OK, closing this at last. Thanks everyone!

OK, I just tested this on my recent CVS build: I can uninstall the non-selected language pack (I tried en-US), it is removed from the list, and it gets un-listed in chrome.rdf's locale:root sequence. The components of it are still in chrome.rdf, but I think that's intended, as well as the files still being left in the directory. verifying