Closed Bug 237407 Opened 20 years ago Closed 20 years ago

[FIX] classic theme scrollbars + buttons leak into modern, when starting with profile manager

Categories

(SeaMonkey :: Themes, defect)

x86
All
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.7final

People

(Reporter: bernd_mozilla, Assigned: benjamin)

References

Details

(Whiteboard: Fix in hand, please don't comment without cause)

Attachments

(4 files)

I couldnt find the dupe. :-(
between 2004-03-09 and 2004-03-10 the buttons in the modern stopped to be
skinned. They look like the default buttons but not blue, and dont have rounded
corners.
Attached image screenshot
Hmm.. this seems to workforme with a 2004-03-11-09 build on Linux.  Is this a
native theming issue of some sort?

Frankly, of the checkins in that range bsmedberg's looks the most suspect...
can anyone confirm.  marking plus until we know more
Flags: blocking1.7b? → blocking1.7b+
removing blocking, this is some strange interference with my debug build. When I
download a fresh build it shows up as the picture attached if I run then my
debug build, which doesnt show the problem, and leave it and then restart the
nightly everything is working as desired.
Flags: blocking1.7b+
I can confirm. this is a problem with the modern theme on windowsXP and 2k (what
I've tested so far). 
Attached image Another example
I see this as well, and confirmed with Asa.

I agree this should block 1.7b.  While it's not a functionality issue from what
I can see, by definition 1.7b should technically act somewhat like a final
release.  Not to mention it really clashes with my desktop.
reassigning to bsmedberg... can you look at your changes in the aforementioned
timeframe and make sure that nothing in modern is getting clobbered?
Assignee: themes → bsmedberg
Using DOM inspector on the search button in Asa's build shows that it's applying
style rules from chrome://global/skin/button.css that are clearly the style
rules from the classic version of that file, even though loading
chrome://global/skin/button.css by typing it in the URL bar loads the modern
version of the file.
fresh build and profile: bug is on Linux too
OS: Windows XP → All
one more thing... in modern theme it's now impossible to resize fonts on
personal toolbar via userChrome.css
Asa can't reproduce anymore if he starts with -P <profilename> instead of going
through the profile manager (which uses classic).  I'm suspecting something's
being cached, although I still can't reproduce with the profile manager...
(The reason I can't reproduce may be that my build is too old -- I'm rebuilding
my Seamonkey tree.)

Bug 233160 seems like the most likely cause.

color is cached - fonts are still microscopic when starting with -P
This may be due to the LayoutStylesheetCache using the CSS loader as a service.
 I'll try changing it to a member variable and dropping it in ::Observe.
Attachment 140978 [details] [diff] also added GetService in a bunch of other places as well.

However, I still can't reproduce the bug, even with a new build...
on Linux:
-scrollbars aren't skinned either.
-in mailnews composer, there's the classic greenish "shadow-color" to the left
and top of the "From" and "Attachment" areas.
-checkboxes are beige/grey (but radio-butons are modern blue'ish!)
I don't see square buttons though.

Dbaron: delete XUL.mfl before starting, if you haven't already..
i can reproduce 100% reliably by creating a new profile
Anyway, I can reproduce the bug, and my theory about its cause was incorrect.
yeah, I think I missed a cache-flush somewhere... looking
Status: NEW → ASSIGNED
But backing out bsmedberg's whole checkin does fix the problem.
This is being caused by my new observer mechanism for flushing chrome skin
caches... the nsLayoutStylesheetCache is racing to reload the scrollbars
stylesheet before the xul prototype stylesheet cache has gotten the "flush"
observer notification. I think I can solve this with a simple lazy-loading
mechanism.
Comment on attachment 144223 [details] [diff] [review]
make the prototype cache actually QI to nsIObserver, and lazy-load the stylesheets from the layoutstylesheetcache

dbaron, there's only one small concern I hadn't thought about initially... the
observer service is holding a strong reference to the xul prototype cache...
will that cause problems at shutdown? I could add nsSupportsWeakReference to
the mix.
Attachment #144223 - Flags: superreview?(dbaron)
Attachment #144223 - Flags: review?(dbaron)
This needs to block 1.7 one way or another. Is 1.7b gone yet?
Flags: blocking1.7b?
Flags: blocking1.7+
Comment on attachment 144223 [details] [diff] [review]
make the prototype cache actually QI to nsIObserver, and lazy-load the stylesheets from the layoutstylesheetcache

Please do add the weak reference stuff (inherit from nsSupportsWeakReference,
change the NS_IMPL_ISUPPORTS macro, change the magic PRBool parameters).  (Not
implementing nsIObserver meant that the observer service wasn't holding on to
the observer, right -- so the lifetime would be changed by this patch?)
Comment on attachment 144223 [details] [diff] [review]
make the prototype cache actually QI to nsIObserver, and lazy-load the stylesheets from the layoutstylesheetcache

oh, it's already a service, so never mind...
Attachment #144223 - Flags: superreview?(dbaron)
Attachment #144223 - Flags: superreview+
Attachment #144223 - Flags: review?(dbaron)
Attachment #144223 - Flags: review+
Comment on attachment 144223 [details] [diff] [review]
make the prototype cache actually QI to nsIObserver, and lazy-load the stylesheets from the layoutstylesheetcache

<leaf> well, i will talk with drivers.
<leaf> we might delay the release if we have a fix.
<leaf> i've made a tag, but i can update it with your changes if that's the way
they want to go
Attachment #144223 - Flags: approval1.7b?
Attachment #144223 - Flags: approval1.7?
Flags: blocking1.7b? → blocking1.7b-
Attachment #144223 - Flags: approval1.7b?
It may be a different bug, but I have seen the behaviour on some of our
university computers running (I think) Mozilla 1.5 on Win2000

These computers have a local c: and also a network n: which is the same whatever
computer you are on. The documents and settings data is on n:, but mozilla is
installed to c:.

Anyway, if you move from one computer to another, then it brings up the classic
skin, with mostly modern icons except the print icon which is a different image
(I can't remember what) tiled in the area where the print icon should be.

If this info is useful I can get more info on the computers in question, but
they are difficult to get access to outside of class unfortunatly.
Summary: modern theme doesnt skin the button's menu and menu backgrounds → modern theme doesn't skin the button's menu and menu backgrounds
Got this bug too and that's what I've done. Click on uninstall Classic on the
themes preferences and restart. And everything is ok now. HTH.
Severity: normal → critical
Summary: modern theme doesn't skin the button's menu and menu backgrounds → [FIX] classic theme scrollbars + buttons leak into modern, when starting with profile manager
Whiteboard: Fix in hand, please don't comment without cause
Target Milestone: --- → mozilla1.7final
Everything ok... since next start. :-(
*** Bug 237996 has been marked as a duplicate of this bug. ***
Confirming on Windows NT 4.0. 

[X] Disable XUL Cache (Debug/Networking pef panel) is my work around for now.
For me it looks like it has something to do with the quick launch...

I work with two profiles, if I have the problem I selected the modern theme
again, quit quick launch and start mozilla again. then it was ok. After a
restart of mozilla the problem was there again.
Now I disabled quick launch permanently and the probleme was gone - till now...
Fabrice, HJ, Markus and everybody else:
Didn't you read what the whiteboard says and already said when you commented:
"Fix in hand, please don't comment without cause"?
Or at least use your common-sense and deduce from the existence of a reviewed
patch that the cause of this problem might already be known?
(disable XUL cache might help, but I don't think this is recommended..)

Everybody else about to add a comment:

### NO NEED TO COMMENT ### 
except for when you're absolutely sure your comment is important. 
A FIX DOES ALREADY EXIST!
(In reply to comment #36)
> (disable XUL cache might help, but I don't think this is recommended..)

Developer like me need to disable XUL cache, or mozilla won't use my changes!

> except for when you're absolutely sure your comment is important. 

I received over 200 e-mails about this problem, and pointed people to this bug
and another bug, disabling the XUL cache simply works. So it is important for
us. I do this often, and they sometimes land my work arounds in mozillaZine
forums, so quit the bs.

.
*** Bug 238202 has been marked as a duplicate of this bug. ***
Comment on attachment 144223 [details] [diff] [review]
make the prototype cache actually QI to nsIObserver, and lazy-load the stylesheets from the layoutstylesheetcache

a=asa (on behalf of drivers) for checkin to 1.7
Attachment #144223 - Flags: approval1.7? → approval1.7+
I checked this in, but it's causing tinderboxen to go orange doing Tp and Ts. I
was able to reproduce the crash:

###!!! ASSERTION: document demuxed from FastLoad file more than once?:
'entry->mNextSegmentOffset != 0', file
/opt/mozillatrunk/mozilla/xpcom/io/nsFastLoadFile.cpp, line 557
Break: at file /opt/mozillatrunk/mozilla/xpcom/io/nsFastLoadFile.cpp, line 557
###!!! ASSERTION: unable to create tokens without an allocator.:
'mTokenAllocator', file /opt/mozillatrunk/mozilla/htmlparser/src/CNavDTD.cpp,
line 561
Break: at file /opt/mozillatrunk/mozilla/htmlparser/src/CNavDTD.cpp, line 561
 
Program /opt/mozillatrunk/dbg-i686-pc-linux-gnu/dist/bin/mozilla-bin (pid =
19296) received signal 11.
Stack:

#4  <signal handler called>
#5  0x41441c7e in nsXULDocument::InsertElement(nsIContent*, nsIContent*) (
    aParent=0x834b430, aChild=0x0)
    at /opt/mozillatrunk/mozilla/content/xul/document/src/nsXULDocument.cpp:4120
#6  0x4143ea29 in nsXULDocument::OverlayForwardReference::Resolve() (
    this=0x834b430)
    at /opt/mozillatrunk/mozilla/content/xul/document/src/nsXULDocument.cpp:3628
#7  0x41435b67 in nsXULDocument::ResolveForwardReferences() (this=0x8323658)
    at /opt/mozillatrunk/mozilla/content/xul/document/src/nsXULDocument.cpp:1362
#8  0x4143c830 in nsXULDocument::ResumeWalk() (this=0x8323658)
    at /opt/mozillatrunk/mozilla/content/xul/document/src/nsXULDocument.cpp:3030
#9  0x41434115 in nsXULDocument::EndLoad() (this=0x8323658)
    at /opt/mozillatrunk/mozilla/content/xul/document/src/nsXULDocument.cpp:733
#10 0x4142c416 in XULContentSinkImpl::DidBuildModel() (this=0x83230c0)
    at 
#11 0x42248fcd in CNavDTD::DidBuildModel(unsigned, int, nsIParser*,
nsIContentSink*) (this=0x8316750, anErrorCode=0, aNotifySink=1, aParser=0x837bea8,
    aSink=0x83230c0)
    at /opt/mozillatrunk/mozilla/htmlparser/src/CNavDTD.cpp:707
#12 0x42262134 in nsParser::DidBuildModel(unsigned) (this=0x837bea8,
    anErrorCode=0)
    at /opt/mozillatrunk/mozilla/htmlparser/src/nsParser.cpp:1245
#13 0x42263177 in nsParser::ResumeParse(int, int, int) (this=0x837bea8,
    allowIteration=1, aIsFinalChunk=1, aCanInterrupt=1)
    at /opt/mozillatrunk/mozilla/htmlparser/src/nsParser.cpp:1791
#14 0x42264caa in nsParser::OnStopRequest(nsIRequest*, nsISupports*, unsigned)
    (this=0x837bea8, request=0x82574b0, aContext=0x0, status=0)
    at /opt/mozillatrunk/mozilla/htmlparser/src/nsParser.cpp:2472
#15 0x42432efe in nsDocumentOpenInfo::OnStopRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x8258920, request=0x82574b0, aCtxt=0x0, aStatus=0)
    at /opt/mozillatrunk/mozilla/uriloader/base/nsURILoader.cpp:352
#16 0x4220161e in nsCachedChromeChannel::HandleStopLoadEvent(PLEvent*) (
    aEvent=0x8316558)
    at /opt/mozillatrunk/mozilla/rdf/chrome/src/nsChromeProtocolHandler.cpp:474
#17 0x40986364 in PL_HandleEvent (self=0x8316558)
    at /opt/mozillatrunk/mozilla/xpcom/threads/plevent.c:671
#18 0x40986205 in PL_ProcessPendingEvents (self=0x8186c18)
    at /opt/mozillatrunk/mozilla/xpcom/threads/plevent.c:606

I'm backing this out... brendan, could you take a look and see if I've made an
obvious mistake somewhere? If not, I should probably just back out bug 233160
and examine this in 1.8a.
*** Bug 238268 has been marked as a duplicate of this bug. ***
If I only flush the XUL prototype skin cache, instead of the full cache, on
profile-switch, we don't crash. This matches the logic pre-bug 233160. I
changed that behavior because we keep content packages in the profile, which
would need flushing.

Let's get this in as it stands, and examine the fastload logic in 1.8a. Maybe
the fastload service needs profile-switching logic? It seems that it would
write to the wrong XUL.mfasl on a profile-switch...
Comment on attachment 144507 [details] [diff] [review]
supplementary patch to prevent crashes

This patch is supplementary to attachment 144223 [details] [diff] [review], they need to land together.
Attachment #144507 - Flags: superreview?(brendan)
Attachment #144507 - Flags: review?(brendan)
*** Bug 238333 has been marked as a duplicate of this bug. ***
Comment on attachment 144507 [details] [diff] [review]
supplementary patch to prevent crashes

FastLoad certainly needs to be told to close any existing file located in the
profile being switched from.  I don't think it needs to observe profile-change
notices, itself, though -- something higher level should do that for it and use
its existing API to keep things consistent.

/be
Attachment #144507 - Flags: superreview?(brendan)
Attachment #144507 - Flags: superreview+
Attachment #144507 - Flags: review?(brendan)
Attachment #144507 - Flags: review+
Attachment #144507 - Flags: approval1.7?
Comment on attachment 144507 [details] [diff] [review]
supplementary patch to prevent crashes

a=mkaply
Attachment #144507 - Flags: approval1.7? → approval1.7+
checked in
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
*** Bug 238441 has been marked as a duplicate of this bug. ***
*** Bug 238426 has been marked as a duplicate of this bug. ***
*** Bug 238635 has been marked as a duplicate of this bug. ***
*** Bug 238702 has been marked as a duplicate of this bug. ***
*** Bug 238697 has been marked as a duplicate of this bug. ***
*** Bug 239449 has been marked as a duplicate of this bug. ***
*** Bug 240006 has been marked as a duplicate of this bug. ***
*** Bug 240045 has been marked as a duplicate of this bug. ***
*** Bug 241138 has been marked as a duplicate of this bug. ***
*** Bug 241266 has been marked as a duplicate of this bug. ***
*** Bug 241297 has been marked as a duplicate of this bug. ***
(In reply to comment #11)
> one more thing... in modern theme it's now impossible to resize fonts on
> personal toolbar via userChrome.css

I could not find the above bug mentioned elsewhere ... i am using 
Mozilla 1.7b (Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.7b) Gecko/20040426)
w/ the above mentioned problem.

Is there any workaround -- till i wait for the fix or revert to some previous
version -- so that mozilla would start respecting userChrome.css?

Should i file a separate bug?
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: