Closed
Bug 190174
Opened 22 years ago
Closed 21 years ago
Migrate profile, unable to switch from Classic to Modern or Modern to Classic
Categories
(Core Graveyard :: Skinability, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.4final
People
(Reporter: nbaca, Assigned: shliang)
References
Details
(Keywords: access, regression, Whiteboard: [adt2])
Attachments
(4 files, 1 obsolete file)
3.07 KB,
text/html
|
Details | |
942 bytes,
patch
|
Details | Diff | Splinter Review | |
960 bytes,
patch
|
ccarlen
:
review+
|
Details | Diff | Splinter Review |
1.96 KB,
patch
|
jesup
:
review+
jag+mozilla
:
superreview+
jesup
:
approval1.4+
|
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.
Reporter | ||
Comment 1•22 years ago
|
||
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
Keywords: nsbeta1
Comment 2•22 years ago
|
||
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.
Keywords: access
Comment 3•22 years ago
|
||
> 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.
Comment 4•22 years ago
|
||
see also bug 190336.
Updated•22 years ago
|
QA Contact: pmac → gbush
Comment 6•21 years ago
|
||
*** Bug 202816 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 7•21 years ago
|
||
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.
Updated•21 years ago
|
OS: MacOS X → Windows XP
Hardware: Macintosh → PC
Comment 8•21 years ago
|
||
Comment 9•21 years ago
|
||
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
Comment 10•21 years ago
|
||
i was able to repro this with a freshly migrated profile on win2k, but unable to do so with an old migrated profile.
Comment 11•21 years ago
|
||
adt: nsbeta1+/adt2 Bug has morphed to cover windows now.
Comment 12•21 years ago
|
||
updating summary
Summary: Migrate profile, unable to switch from Classic to Modern → Migrate profile, unable to switch from Classic to Modern or Modern to Classic
Assignee | ||
Comment 13•21 years ago
|
||
add defaults/profile/chrome to package list
Attachment #123940 -
Flags: review?(ssu)
Comment 14•21 years ago
|
||
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 15•21 years ago
|
||
Comment on attachment 123940 [details] [diff] [review] patch sr=jag if you take the ns parts out (not needed).
Attachment #123940 -
Flags: superreview+
Attachment #123940 -
Flags: approval1.4?
How does this patch fix the bug? It looks like the only files in that directory are examples.
Comment 17•21 years ago
|
||
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.
Comment 19•21 years ago
|
||
Ah, that would explain.
Assignee | ||
Comment 20•21 years ago
|
||
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
Comment 21•21 years ago
|
||
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 22•21 years ago
|
||
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?
Comment 23•21 years ago
|
||
theme switching needs to be fixed for 1.4 final, marking as a blocker. cc asa, but I'm confident he will agree.
Flags: blocking1.4+
Target Milestone: --- → mozilla1.4final
Comment 24•21 years ago
|
||
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 25•21 years ago
|
||
Comment on attachment 123940 [details] [diff] [review] patch hold off on this. comments coming.
Attachment #123940 -
Flags: approval1.4?
Comment 26•21 years ago
|
||
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 27•21 years ago
|
||
Comment on attachment 123940 [details] [diff] [review] patch no go on this.
Attachment #123940 -
Attachment is obsolete: true
Comment 28•21 years ago
|
||
Comment 29•21 years ago
|
||
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.
Assignee | ||
Comment 30•21 years ago
|
||
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 31•21 years ago
|
||
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+
Assignee | ||
Comment 32•21 years ago
|
||
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.
Attachment #124469 -
Flags: superreview?(jaggernaut)
Attachment #124469 -
Flags: review?(ccarlen)
Comment 33•21 years ago
|
||
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 34•21 years ago
|
||
Comment on attachment 124469 [details] [diff] [review] also fix file names in GetProfileRoot sr=jag
Attachment #124469 -
Flags: superreview?(jaggernaut) → superreview+
Assignee | ||
Comment 35•21 years ago
|
||
checked in
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 36•21 years ago
|
||
is this fixed on trunk and branch, or just trunk?
Updated•21 years ago
|
Attachment #124469 -
Flags: approval1.4?
Comment 37•21 years ago
|
||
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+
Updated•21 years ago
|
Attachment #124469 -
Flags: approval1.4? → approval1.4+
Comment 38•21 years ago
|
||
This has been approved for 1.4. When it's checked into 1.4, please add the keyword 'fixed1.4' .
Comment 39•21 years ago
|
||
on XP I am seeing this fixed on trunk (2003060305) but not branch (2003060305) looks ok on Mac OS X branch
Comment 40•21 years ago
|
||
a=adt to land this fix on the 1.4 branch. Please add the fixed1.4 keyword once you land this.
Comment 41•21 years ago
|
||
clearing my status whiteboard comment, the fix I rejected was not checked in. (another fix was done.)
Whiteboard: [adt2] [need new fix] → [adt2]
Comment 42•21 years ago
|
||
verified on XP branch 2003060905 (and also on Mac/Linux branches/trunks)
Status: RESOLVED → VERIFIED
Keywords: fixed1.4 → verified1.4
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•