Closed Bug 190174 Opened 17 years ago Closed 17 years ago
Migrate profile, unable to switch from Classic to Modern or Modern to Classic
3.07 KB, text/html
942 bytes, patch
|Details | Diff | Splinter Review|
960 bytes, patch
|Details | Diff | Splinter Review|
1.96 KB, patch
|Details | Diff | Splinter Review|
Trunk build 2003-01-21: Mac 10.1.5 (trunk 2002-12-18 and 7.01 builds) Overview: Migrate a profile and the current skin is Classic. Change to Modern but it never changes. Steps to reproduce: 1. Migrate a profile (currently, it's classic by default) 2. Select View|Apply Theme|Modern and it prompts that the changes will not take effect until after a restart. 3. Exit/Restart Actual Results: It still displays the classic theme. I tried deleting the "XUL Fast Load File" and "localstore.rdf" but the problem still occured. Interesting...New profiles don't have this problem, can switch between classic and modern. Expected Results: Switching from the Classic to Modern theme should be possilbe. Additional Information: 1. 2003-01-21: WinMe, ok.
Marking nsbeta1 because in a migrated profile you should be able to switch from a Classic skin to Modern. This is very inconvenient for users with accessibility issues since there are so many regressions in the classic skin. If Modern became the default skin for the Mac then the accessibility issue would be of less concern.
Assignee: andreww → varga
esther found that this was a problem with 12/16 trunk builds. patty / grace, do you know if there's a bug filed for this already? nbaca and i couldn't find one.
> Interesting...New profiles don't have this problem, can switch between classic and modern. That is interesting - Does this bug exist if the profile was migrated long ago, or is it only with freshly migrated profiles? Reason I mention freshly migrated is that the Mach-O build doesn't migrate 4.x profiles.
see also bug 190336.
*** Bug 202816 has been marked as a duplicate of this bug. ***
Renominating since this is a regression and a basic feature that should work. Switching themes on the Mac appears to work but Windows continues to have the problem.
OS: MacOS X → Windows XP
Hardware: Macintosh → PC
Did some testing with migrated profiles - some going through activation process, some not- see attached from comment #3 Does this bug exist if the profile was migrated long ago, or is it only with freshly migrated profiles? I have an previously migrated profile (which I keep backed up) which was activated and which was changed to classic theme
i was able to repro this with a freshly migrated profile on win2k, but unable to do so with an old migrated profile.
adt: nsbeta1+/adt2 Bug has morphed to cover windows now.
Summary: Migrate profile, unable to switch from Classic to Modern → Migrate profile, unable to switch from Classic to Modern or Modern to Classic
add defaults/profile/chrome to package list
Comment on attachment 123940 [details] [diff] [review] patch this patch is necessary to get the chrome files packaged up. It has a side effect of fixing this bug, but if we ever stopped packaging this chrome dir, the bug will come back. r=ssu for this patch, but you should still try to find out what's causing this bug. ps. I just realized that you shoulnd't need to do the same patch to the ns tree because the same moz packages- files will be run on the ns tree.
Attachment #123940 - Flags: review?(ssu) → review+
Comment on attachment 123940 [details] [diff] [review] patch sr=jag if you take the ns parts out (not needed).
How does this patch fix the bug? It looks like the only files in that directory are examples.
Note: it looks like the Mac install packager already packages up those two css files (don't know how), but that's why we don't see this bug on Mac.
packages-mac is defunct, and the Mach-O build just packages everything, IIRC.
Ah, that would explain.
nsChromeRegistry::GetProfileRoot fails if the chrome directory doesn't exist in the user profile directory and defaults/profile/chrome isn't in the install directory
So nsProfileDirSericeProvider::GetFile shouldn't call EnsureProfileFileExists for sApp_UserChromeDirectory (since that's not what the caller expects), or the caller should be fixed to assume the directory will exist after a successful call to NS_GetSpecialDirectory (less robust, only valid if the installer packages the defaults/profile/chrome directory, see patch), and should instead check for the existence of these two files before copying them over. Though it turns out these two files get copied over anyway, and not by this code.
Comment on attachment 123940 [details] [diff] [review] patch which package did these get added to? I'm thinking about the browser only scenario. if these got added to [mail], that would be a problem, right?
theme switching needs to be fixed for 1.4 final, marking as a blocker. cc asa, but I'm confident he will agree.
Target Milestone: --- → mozilla1.4final
shuehan tells me that she's added these lines to the "[deflenus]" section. what about non en-US builds? why not under "; chrome stuff"? shuehan, can you discuss with ssu, who knows these packaging issues best?
Comment on attachment 123940 [details] [diff] [review] patch hold off on this. comments coming.
of course, I agree, we have to fix skin switching. but maybe adding these two files back isn't what we want. on a tip from samir/shuehan, I queried bugzilla and found that they were removed on purpose. see http://bugzilla.mozilla.org/show_bug.cgi?id=110555 instead of relying on the installer to copy over these files, why can't we fix the code to either ensure that defaults/profile/chrome directory exists, or that we handle it better when it doesn't? (I vote for the latter). shuehan wrote: "nsChromeRegistry::GetProfileRoot fails if the chrome directory doesn't exist in the user profile directory and defaults/profile/chrome isn't in the install directory" let's fix that instead
Whiteboard: [adt2] → [adt2] [need new fix]
Comment on attachment 123940 [details] [diff] [review] patch no go on this.
Attachment #123940 - Attachment is obsolete: true
All nsChromeRegistry::GetProfileRoot() is really required to do is to get a file which points to the profile chrome dir and make a URL from that. It should only fail if the directory itself does not exist after calling NS_GetSpecialDirectory(NS_APP_USER_CHROME_DIR). The problem is that EnsureProfileFileExists() is being used here on a directory. nsProfileDirSericeProvider::GetFile() will fail if the directory is there but copying the defaults fails. Really, NS_APP_USER_CHROME_DIR should refer only to the directory, not its contents, too. Then, we define two more directory service props, for each of the two files. GetProfileRoot() could then just call NS_GetSpecialDirectory for each of those, after succeeding on NS_APP_USER_CHROME_DIR. It would also remove some bloat. See bug 37642, comment 57. That bug was back in the days before you could '-' a patch ;-) A simpler fix would be to have nsProfileDirSericeProvider::GetFile() not call EnsureProfileFileExists() and make sure nsChromeRegistry::GetProfileRoot() can't fail on copying the default files.
remove EnsureProfileFileExists() from NS_GetSpecialDirectory(NS_APP_USER_CHROME_DIR) so that chrome directory will be created. i'm not sure any other changes are needed, the default css files don't need to be there for skin switching to work.
Comment on attachment 124298 [details] [diff] [review] remove EnsureProfileFileExists() r=ccarlen. BTW, is the code in nsChromeRegistry::GetProfileRoot() which copies these files from the defaults still useful since those files are no longer present in the defaults/profile dir?
Attachment #124298 - Flags: review+
the code to copy the default files is still useful for non-installed builds - just realized that wasn't working anyway though because the filenames are wrong so changed those.
If you're going to touch GetProfileRoot(), how about this, while you're there: defaultUserContentFile->AppendNative(NS_LITERAL_CSTRING("chrome")); defaultUserContentFile->Clone(getter_AddRefs(defaultUserChromeFile)); defaultUserContentFile->AppendNative(NS_LITERAL_CSTRING("userContent-example.css")); defaultUserChromeFile->AppendNative(NS_LITERAL_CSTRING("userChrome-example.css")); Using Clone() allows you to get rid of the 2nd set of calls to NS_GetSpecialDirectory() and a call to Append(). Also, there shouldn't be any "return rv" in that block - it's all optional. r=ccarlen on what you have since the GetProfileRoot redundancy isn't yours.
Comment on attachment 124469 [details] [diff] [review] also fix file names in GetProfileRoot sr=jag
Attachment #124469 - Flags: superreview?(jaggernaut) → superreview+
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
is this fixed on trunk and branch, or just trunk?
Comment on attachment 124469 [details] [diff] [review] also fix file names in GetProfileRoot conrad r='d this in a comment.
Attachment #124469 - Flags: review?(ccarlen) → review+
Attachment #124469 - Flags: approval1.4? → approval1.4+
This has been approved for 1.4. When it's checked into 1.4, please add the keyword 'fixed1.4' .
on XP I am seeing this fixed on trunk (2003060305) but not branch (2003060305) looks ok on Mac OS X branch
a=adt to land this fix on the 1.4 branch. Please add the fixed1.4 keyword once you land this.
clearing my status whiteboard comment, the fix I rejected was not checked in. (another fix was done.)
Whiteboard: [adt2] [need new fix] → [adt2]
verified on XP branch 2003060905 (and also on Mac/Linux branches/trunks)
You need to log in before you can comment on or make changes to this bug.