191954 - using profile with non-present theme modifies build and profile chrome.rdf, differently

Status: RESOLVED FIXED

(Core Graveyard :: Skinability, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This is a spin-off of bug 191887 of one specific easily-reproducable problem
that is probably the root of some other problems.

If I do the following steps:

 1. install an instance of 1.0.2 and current trunk
 2. Create a profile using 1.0.2 and install LittleMozilla from
http://themes.mozdev.org/themes/littlemozilla.html using the link
javascript:doXPIInstall('http://mozdev.mirrors.nyphp.org/themes/themes/littlemozilla_10.xpi');
 3. Switch that profile to LittleMozilla
 4. Start the 1.0.2 build with that profile, so it uses the LittleMozilla theme.
 5. Start the current trunk build with a normal profile (using, say, Modern).
 6. Run with that profile in the current trunk build (with -P on the command
line, so only the browser is run).

After step 5, all the "selectedSkin" elements in the chrome/chrome.rdf in the
*profile* directory all refer to littlemozilla, as they should, and the
selectedSkin elements in the chrome/chrome.rdf in the *trunk build* directory
all refer to classic, as they should.

After step 6, the selectedSkin element for the *navigator* package in the
*profile* directory now refers to modern (all other packages remain
littlemozilla), and the selectedSkin element for the *global* and *communicator*
packages in the *trunk build* directory now refer to modern (all other packages
remain classic).

This switching behavior can lead to clodern when different build/profile
combinations are used on this build or profile.  (Also, which skin is written in
depends, I think, on some of the ordering stuff that we don't want to depend on,
since it's modern for a current trunk build, but I *believe* it was classic for
a 1.2.1 build.)

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In step 6, "that profile" refers to the profile using LittleMozilla.  The
profile directory mentioned in the results is also the profile using LittleMozilla.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

BTW, the parenthetical sentence in the last paragraph in comment 0 is now bug
191957.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(See bug 191887 comment 2 for a way to reproduce "clodern" of which this bug is
part of the cause.)

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This seems to be intentional behavior in nsChromeRegistry::SelectPackageInProvider.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: a hacky patch

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: same patch, with some more cleanup

This uses the added function in one other place where it could be used, uses
the slightly better error-handling from that location, and removes the unneeded
|aBaseURL| out parameter.

Comment 7

[Dan M]

Comment on attachment 113627 [details] [diff] [review]
same patch, with some more cleanup

r=danm if you're quite certain that mInstallDirChromeDataSource will be
initialized before its use in GetBaseURL. Admittedly it seems likely and all.
But still, an assertion, maybe?

Oh and Scruffy, he's my new and annoying mascot, wanted me to say you probably
meant to spell the word "nonexistent" without making use of the letter A.

+  // XXXldb For the case where the file is nonexistant, we're depending

Blame Scruffy. He's anal. I just do what the voices tell me.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk, 2003-06-01 13:15 PDT.

I didn't bother with the null check since I'd rather leave things consistent
with the treatment of mChromeDataSource, which is initialized in the same
function as mInstallDirDataSource.  I did fix the spelling error, though.