Closed Bug 116306 Opened 23 years ago Closed 22 years ago

Switching themes by switching profiles is broken

Categories

(Core Graveyard :: Skinability, defect, P2)

Product:
Core Graveyard
Component:
Skinability
Platform:
x86
Windows 2000
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED WORKSFORME
Milestone:
mozilla0.9.9

People

(Reporter: ccarlen, Assigned: ccarlen)

References

Details

(Whiteboard: [driver:brendan])

Attachments

(1 file, 1 obsolete file)

Patch to handle a case that was missing. Aha! :)
3.32 KB, patch
dbaron
: review+
bugzilla
: superreview+
Details | Diff | Splinter Review 
Sometime between 2001121703 and 2001121803, this feature got completely broken.
Steps To reproduce:
(1) Have more than 1 profile, each with a different theme.
(2) Turn turbo on in the preferences and exit.
(3) Choose "Navigator" in the system tray and the profile picker will come up
(4) Choose a profile with a different theme and click "Start"
(5) The Navigator window that comes up has completely malformed chrome.

Didn't some live theme switching code go in recently? Could this have wacked the
theme switching which happens on a profile switch (which has worked for a long time)
Here is the check-in for theme switching that went in during that time:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=hyatt%25netscape.com&whotype=match&sortby=Date&hours=2&date=explicit&mindate=12%2F17%2F2001&maxdate=12%2F18%2F2001&cvsroot=%2Fcvsroot
Looking at all check-ins during that time, this seems most likely to have caused
it. Hyatt - Can you look at this?
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.8
-> hyatt
Assignee: ben → hyatt
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Target Milestone: mozilla0.9.8 → mozilla0.9.9
I'd suggest mozilla0.9.8 for this one.  It makes turbo mode pretty much useless.
*** Bug 120361 has been marked as a duplicate of this bug. ***
*** Bug 119553 has been marked as a duplicate of this bug. ***
p2/nsbeta1
Keywords: nsbeta1
Priority: -- → P2
Whiteboard: [driver:brendan]
Target Milestone: mozilla0.9.9 → mozilla0.9.8
So I was being bitten by something that was just never implemented, namely
support for fetching stylesheets on a miss from the XUL cache (when the proto
XUL doc was hit in the cache).
Ignore the XULElement patch.  XULDocument is the only relevant file.
Whiteboard: [driver:brendan] → [driver:brendan] [Fix in hand. Need r/sr/a]
Comment on attachment 65766 [details] [diff] [review]
Patch to handle a case that was missing. Aha! :)

r=dbaron, except you could switch the logic to

if (!IsChromeURI(uri))
  continue;

and avoid an extra indent.
Attachment #65766 - Flags: review+
Comment on attachment 65766 [details] [diff] [review]
Patch to handle a case that was missing. Aha! :)

sr=blake
Attachment #65766 - Flags: superreview+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: [driver:brendan] [Fix in hand. Need r/sr/a] → [driver:brendan]
Yay! It works again. Thanks.
Don't mix two-space indentation with four-space -- respect emporer Watersonus
Maximus when in his Rome!

a=brendan@mozilla.org retroactively.

/be
Note: The checkin to this bug caused bug 121057  (Prefs panel "Advanced|Scripts
& Windows" can only be visited once per session)
Err, I guess I missed comment #8 which nullifies the XULElement part of the
patch.  Sorry about my last comment.  I'll leave this open though as it did
cause a regression.
I spoke too soon that it worked. While the chrome that opens after switching the
profile is not mangled anymore, it's not the right skin. Check out the stack trace:

nsChromeRegistry::LoadProfileDataSource() line 2853
nsChromeRegistry::ConvertChromeURL(nsChromeRegistry * const 0x004c7cf0, nsIURI *
0x033ae480, char * * 0x033ae3f0) line 524 + 8 bytes
nsChromeProtocolHandler::NewChannel(nsChromeProtocolHandler * const 0x004c1c50,
nsIURI * 0x033ae480, nsIChannel * * 0x0012c298) line 619 + 63 bytes
nsIOService::NewChannelFromURI(nsIOService * const 0x004b33d0, nsIURI *
0x033ae480, nsIChannel * * 0x0012c298) line 794 + 31 bytes
NS_OpenURI(nsIChannel * * 0x0012c2d8, nsIURI * 0x033ae480, nsIIOService *
0x004b33d0, nsILoadGroup * 0x00000000, nsIInterfaceRequestor * 0x00000000,
unsigned int 0) line 148 + 20 bytes
NS_OpenURI(nsIInputStream * * 0x0012c428, nsIURI * 0x033ae480, nsIIOService *
0x00000000, nsILoadGroup * 0x00000000, nsIInterfaceRequestor * 0x00000000,
unsigned int 0) line 186 + 49 bytes
CSSLoaderImpl::LoadAgentSheet(CSSLoaderImpl * const 0x033ae6a0, nsIURI *
0x03ddd320, nsICSSStyleSheet * & 0x00000000, int & 65, nsICSSLoaderObserver *
0x00000000) line 1598 + 21 bytes
nsXBLPrototypeResources::FlushSkinSheets() line 140 + 62 bytes
nsXBLPrototypeBinding::FlushSkinSheets(nsXBLPrototypeBinding * const 0x03dda2f0)
line 431 + 11 bytes
FlushScopedSkinSheets(nsHashKey * 0x03dda120, void * 0x03dda2f0, void *
0x00000000) line 383
_hashEnumerate(PLHashEntry * 0x03dda0e0, int 3, void * 0x0012c54c) line 198 + 26
bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x03dda218, int (PLHashEntry *, int,
void *)* 0x1002cbf0 _hashEnumerate(PLHashEntry *, int, void *), void *
0x0012c54c) line 429 + 15 bytes
nsHashtable::Enumerate(int (nsHashKey *, void *, void *)* 0x01e1a340
FlushScopedSkinSheets(nsHashKey *, void *, void *), void * 0x00000000) line 361
+ 21 bytes
nsSupportsHashtable::Enumerate(int (nsHashKey *, void *, void *)* 0x01e1a340
FlushScopedSkinSheets(nsHashKey *, void *, void *), void * 0x00000000) line 190
nsXBLDocumentInfo::FlushSkinStylesheets(nsXBLDocumentInfo * const 0x03ddd4e0)
line 391
FlushScopedSkinStylesheets(nsHashKey * 0x03dc20f0, void * 0x03ddd4e0, void *
0x00000000) line 399
_hashEnumerate(PLHashEntry * 0x03dc1c70, int 0, void * 0x0012c5ec) line 198 + 26
bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x005093c8, int (PLHashEntry *, int,
void *)* 0x1002cbf0 _hashEnumerate(PLHashEntry *, int, void *), void *
0x0012c5ec) line 429 + 15 bytes
nsHashtable::Enumerate(int (nsHashKey *, void *, void *)* 0x01da9360
FlushScopedSkinStylesheets(nsHashKey *, void *, void *), void * 0x00000000) line
361 + 21 bytes
nsSupportsHashtable::Enumerate(int (nsHashKey *, void *, void *)* 0x01da9360
FlushScopedSkinStylesheets(nsHashKey *, void *, void *), void * 0x00000000) line 190
nsXULPrototypeCache::FlushSkinFiles(nsXULPrototypeCache * const 0x00509330) line 422
nsChromeRegistry::FlushCaches() line 1180
nsChromeRegistry::Observe(nsChromeRegistry * const 0x004c7cf4, nsISupports *
0x004b3cd0, const char * 0x0178c918, const unsigned short * 0x0178c8ac) line 3194

nsChromeRegistry::Observe is being called for "profile-before-change" when the
profile is going away and so it calls FlushCaches. Notice that this causes a
chrome load and we end up re-loading the profile data even though we're in the
middle of unloading it. Next time around, on getting a "profile-after-change"
nothing happens because mProfileInitialized is now true.
 
hyatt says it seems as though the flush shouldn't happen until UI is gone, but
hyatt isn't around for the rest of today and for much of tomorrow -- conrad, can
you attempt a fix for 0.9.8?

/be
Could the flushing call be moved to after-change?  I don't really want to have 
to split up the function into a "Flush" and a "Reload", since I'd do two 
crawls of open UI window trees.  

> Could the flushing call be moved to after-change?
Actually, the call to FlushCaches() from Observe("profile-before-change") can
just be done away with. FlushCaches() is going to be called by the next
LoadProfileDataSource() anyway. Just tried it and it appears to work. A little
more testing...
This seems to do it.
need r=/sr=/a=
Keywords: review
Comment on attachment 66112 [details] [diff] [review]
Removes unneeded call to FlushCaches()

r/sr=brendan@mozilla.org, hyatt, can you review also?

/be
Attachment #66112 - Flags: superreview+
r=hyatt

Comment on attachment 66112 [details] [diff] [review]
Removes unneeded call to FlushCaches()

Recording hyatt's review and adding a=brendan@mozilla.org for checkin to the
0.9.8 branch.

/be
Attachment #66112 - Flags: review+
Attachment #66112 - Flags: approval+
Checked into trunk - will check into branch ASAP.
Checked into 0.9.8
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
a long shot.. but can this have caused bug 121860?
No, the line of code removed by the latest checkin only would have been executed
when a profile is shutdown during a profile switch. That currently only happens
with turbo mode and not with changing themes via prefs or the view menu. 
I'm still seeing weird behavior with build 2002012506

I have 3 profiles, each with their own theme,  I am getting duplicate buttons on
my browser- two completely different print icons.

Also on the launch of second profile, activation comes up with blank screen and
option to save the activation.xul file??? not sure if that is related but no
activation screens can be seen
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Have to check this with commercial and its activation window. On my current
mozilla build, it's working fine.
removing keyword since the fix for this has landed on the 0.9.8 branch
Keywords: mozilla0.9.8+
see also bug 11920 in bugscape  for activation activity
at one point on XP testing- I had 3 profiles, 3 themes and could not launch app- 
error as no xbl binding.  I had to clean all profiles and start over
Grace, what platform, is that windows 98?

NT and XP
->0.9.9
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Grace, Can you check this with mozilla, not commercial? I'd like to separate out
the activation window problems.
nsbeta1+ per Nav triage team, but Dave needs to check for possible dup.
Keywords: nsbeta1 → nsbeta1+
tested mozilla
3 profiles - 3 themes (downloaded eskimo to try)
I did not see any weirdness in chrome except after using profile with specific 
theme, next time ProfileManager comes up, it has that theme
I thought mozilla PM always came up classic...but I may be wrong.

Regular turbo tests passed
Grace: Thanks, knowing that this happens only in commercial helps narrow it down
a bit.

> next time ProfileManager comes up, it has that theme
> I thought mozilla PM always came up classic...but I may be wrong.

It will use the theme that was in use last. See bug 99387 for why.
Blocks: 106664
->ccarlen
Assignee: hyatt → ccarlen
Status: REOPENED → NEW
No longer blocks: 115520
Comment on attachment 66112 [details] [diff] [review]
Removes unneeded call to FlushCaches()

obsoleting old patch.
Attachment #66112 - Attachment is obsolete: true
I couldn't reproduce this one on my windows 2k (branch build: 
2002-05-29-08-1.0.0).
Grace, are you able to reproduce this in the recent build?
I tested again today and cannot reproduce either
If no one can reproduce this, can you mark it Worksforme.  sspitzer fixed a
theme related bug where you could get in a state where the theme was a mixture
of modern and classic.  He fixed this sometime in April so perhaps that fixed
this bug.
This bug seems to rely on multiple profiles and Quick Launch, a combination not
supported in recent releases.
marking nsbeta1- for Mach V per ADT.
Keywords: nsbeta1+ → nsbeta1-
Recent changes in the QuickLaunch model have obviated all other related
multi-profile bugs, please retest this with the pref
"browser.turbo.singleProfileOnly" set to false.
marking worksforme
Have not seen on builds for a long time- retested this morning with branch build 
and turbo pref set appropriately for using multiple profiles
Status: NEW → RESOLVED
Closed: 23 years ago → 22 years ago
Resolution: --- → WORKSFORME
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: