71194 - Front-end for Skin Uninstaller

Status: VERIFIED FIXED

(Core Graveyard :: Skinability, defect)

Description

[:deanis74]

Not sure if this goes to prefs or skinability, so taking my best guess.

Bug 59558 added support in the chrome registry for uninstalling themes.  Now we
need a front-end, I would think in the Themes preference panel.

I started on this, but couldn't tell if the actual chrome registry method was
getting called.  Here's what I have so far...

pref-themes.xul
---------------
1. add something along the lines of:

  <button id="uninstallSkin" value="Remove Theme" accesskey="R"
oncommand="uninstallSkin();" />

below the <button id="applySkin"...> item.  The value and accesskey attributes
should be pulled from the string bundle.

pref_themes.js
--------------
1. themeSelect() should change the label of the uninstallSkin button when a skin
is selected.  It already does this for the applySkin button, in the last four
lines of the function, so this should be straight-forward.

2. Add an uninstallSkin function, something along the lines of:

function uninstallSkin()
{
  var tree = document.getElementById( "skinsTree" );
  var selectedSkinItem = tree.selectedItems[0];
  var skinName = selectedSkinItem.getAttribute( "name" );
  chromeRegistry.uninstallSkin( skinName, DEBUG_USE_PROFILE, false ); 
  chromeRegistry.refreshSkins();
}

But, as I said, I couldn't tell if chromeRegistry.uninstallSkin() was getting
called or not.

Hopefully someone can take this and run with it.

Comment 1

[sairuh (rarely reading bugmail)]

i'm guessing ->skinability.

Comment 2

[David Hyatt]

YOu don't need to refreshSkins if the uninstalled skin wasn't in use.

Comment 3

[:deanis74]

Does it hurt to always do it?  Or should there be some sort of check, and it
only refreshes if it was currently in use.  Actually, we probably shouldn't
allow deleting a skin that's currently in use.

Comment 4

[David Hyatt]

Yes.  File a bug on me to implement a new API to ask whether a skin is in use.

Comment 5

[:deanis74]

Done, bug 71226

Comment 6

[:deanis74]

The call in my code snippet to nsChromeRegistry.uninstallSkin() is returning:

 line 0: uncaught exception: [Exception... "Component returned failure code: 0x8
0004005 (NS_ERROR_FAILURE) [nsIChromeRegistry.uninstallSkin]"  nsresult: "0x8000
4005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://communicator/content/p
ref/pref-themes.js :: uninstallSkin :: line 39"  data: no]

Comment 7

[David Hyatt]

YOu have a bogus third argument to uninstallSKin.  It only takes two args.

Comment 8

[:deanis74]

Changed that, and I still get the same error.  I put verification messages
around it just to be sure:

alert( "uninstallSkin(" + skinName + ", " + DEBUG_USE_PROFILE + ")" );
  chromeRegistry.uninstallSkin( skinName, DEBUG_USE_PROFILE ); 
alert( "finish" );

The first alert reads "uninstallSkin(blue/1.0, true)".  I never see the "finish"
alert, I assume due to the error.

Comment 9

[David Hyatt]

Blue was likely installed in your install dir, which means the uninstall call on
the profile will fail.

Comment 10

[:deanis74]

Not sure about that, but I just installed the Alfred Skin and I can't uninstall
that, either.  Same error.

Comment 11

[David Hyatt]

Hmmm.  Are you sure you have a build with my changes?

Comment 12

[David Hyatt]

Mine.

Comment 13

[David Hyatt]

Fixed.

Comment 14

[Peter Lairo]

any idea when this will appear in a nightly?

Also, will the uninstaller remove skins that were installed from the N6 themes
site (e.g. sky pilot, modern - mozillium)? These seem to behave differently (no
preview pic in prefs).

Comment 15

[:deanis74]

Looks very nice, but the Uninstall button is always disabled. I just installed a
new theme (Thinice) but no go.

Comment 16

[:deanis74]

I still can't get this to work (Win2000).  I just took a fresh build and
re-installed Thinice.  The Uninstall Theme button remains disabled.  I'm
re-opening this until someone can tell me how to get the button to enable.

Comment 17

[David Hyatt]

This totally works for me.  I'm guessing that ThinIce is not a real theme, but
was made as an XPI placed improperly into your install dir.

Anyway, I tested this with every theme on theme park, and they all worked on
WIn2000.

Comment 18

[:deanis74]

Dave, how can I check if this, and the other three or four themes I've installed 
from x.themes.org but can't be uninstalled, is a "real theme" or not?

Comment 19

[Peter Lairo]

using build 2001-03-12 (others crash), win98, 256MB RAM

I still cannot uninstall any of my themes (alfred, greymodern, etc.). 

This bug should not be done until the front-end actually works. With all the
negative feedback, frankly, I don't see how this can be marked fixed :(

Comment 20

[:deanis74]

Wait, I get this, and I get what Hyatt's saying!  (Sometimes it just takes a 
little while...)  I installed Sky Pilot from the Theme Park and it installed 
into my personal profile directory.  When I install anything from x.themes.org, 
it installs into the bin/chrome directory.  Marking verified based on this.

More importantly, how do we let people know how to properly package their 
skins??

Comment 21

[David Hyatt]

The prefs UI covers only profile-based themes.  It does not allow the
uninstallation of any themes that were placed into your install directory.

Comment 22

[Boris Zbarsky [:bzbarsky]]

*** Bug 58706 has been marked as a duplicate of this bug. ***

Comment 23

[Peter Lairo]

and how does one uninstall a theme that installed itself into to install
directory (like almost all currnt themes)? Do we really need another bug that
actually uninstalls themes (regardless of install location)?

Comment 24

[:deanis74]

No, I think we need a bug detailing that we shouldn't allow themes to be 
installed anywhere but the profile directory.

Comment 25

[Peter Lairo]

Done, see bug 74690 - We shouldn't allow themes to be installed anywhere but the
profile directory.

Although that doesn't solve the problem of users already having themes in the
install directory (with references in their prefs.js files) :(