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

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Bernd]

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.

Comment 1

[Bernd]

Attachment: screenshot

Comment 2

[Bernd]

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=03%2F09%2F2004+08%3A00%3A00&maxdate=03%2F10%2F2004+08%3A00%3A00&cvsroot=%2Fcvsroot

we shouldnt ship 1.7b with the modern theme horked.

Comment 3

[Boris Zbarsky [:bzbarsky]]

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...

Comment 4

[chris hofmann]

can anyone confirm.  marking plus until we know more

Comment 5

[Bernd]

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.

Comment 6

[Asa Dotzler [:asa]]

I can confirm. this is a problem with the modern theme on windowsXP and 2k (what
I've tested so far). 

Comment 7

[Robert Accettura [:raccettura]]

Attachment: 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.

Comment 8

[Daniel (Leaf) Nunes]

reassigning to bsmedberg... can you look at your changes in the aforementioned
timeframe and make sure that nothing in modern is getting clobbered?

Comment 9

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

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.

Comment 10

[R.K.Aa.]

fresh build and profile: bug is on Linux too

Comment 11

[R.K.Aa.]

one more thing... in modern theme it's now impossible to resize fonts on
personal toolbar via userChrome.css

Comment 12

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

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...

Comment 13

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

(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.

Comment 14

[R.K.Aa.]

color is cached - fonts are still microscopic when starting with -P

Comment 15

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

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.

Comment 16

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

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...

Comment 17

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

attachment 140978 [details] [diff] [review], that is

Comment 18

[R.K.Aa.]

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..

Comment 19

[R.K.Aa.]

i can reproduce 100% reliably by creating a new profile

Comment 20

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

Anyway, I can reproduce the bug, and my theory about its cause was incorrect.

Comment 21

[Benjamin Smedberg]

yeah, I think I missed a cache-flush somewhere... looking

Comment 22

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

But backing out bsmedberg's whole checkin does fix the problem.

Comment 23

[Benjamin Smedberg]

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 24

[Benjamin Smedberg]

Attachment: make the prototype cache actually QI to nsIObserver, and lazy-load the stylesheets from the layoutstylesheetcache

Comment 25

[Benjamin Smedberg]

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.

Comment 26

[Benjamin Smedberg]

This needs to block 1.7 one way or another. Is 1.7b gone yet?

Comment 27

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

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 28

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

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...

Comment 29

[Benjamin Smedberg]

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

Comment 30

[Ian Tommins ('thelem')]

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.

Comment 31

[Fabrice Bonny]

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.

Comment 32

[Fabrice Bonny]

Everything ok... since next start. :-(

Comment 33

[Stefan Borggraefe]

*** Bug 237996 has been marked as a duplicate of this bug. ***

Comment 34

[HJ]

Confirming on Windows NT 4.0. 

[X] Disable XUL Cache (Debug/Networking pef panel) is my work around for now.

Comment 35

[Markus Jung]

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...

Comment 36

[Andreas Kunz]

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!

Comment 37

[HJ]

(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.

.

Comment 38

[R.K.Aa.]

*** Bug 238202 has been marked as a duplicate of this bug. ***

Comment 39

[Asa Dotzler [:asa]]

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

Comment 40

[Benjamin Smedberg]

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.

Comment 41

[R.K.Aa.]

*** Bug 238268 has been marked as a duplicate of this bug. ***

Comment 42

[Benjamin Smedberg]

Attachment: supplementary patch to prevent crashes

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 43

[Benjamin Smedberg]

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.

Comment 44

[R.K.Aa.]

*** Bug 238333 has been marked as a duplicate of this bug. ***

Comment 45

[Brendan Eich [:brendan]]

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

Comment 46

[Mike Kaply [:mkaply]]

Comment on attachment 144507 [details] [diff] [review]
supplementary patch to prevent crashes

a=mkaply

Comment 47

[Benjamin Smedberg]

checked in

Comment 48

[R.K.Aa.]

*** Bug 238441 has been marked as a duplicate of this bug. ***

Comment 49

[R.K.Aa.]

*** Bug 238426 has been marked as a duplicate of this bug. ***

Comment 50

[R.K.Aa.]

*** Bug 238635 has been marked as a duplicate of this bug. ***

Comment 51

[R.K.Aa.]

*** Bug 238702 has been marked as a duplicate of this bug. ***

Comment 52

[R.K.Aa.]

*** Bug 238697 has been marked as a duplicate of this bug. ***

Comment 53

[Stefan [:stefanh]]

*** Bug 239449 has been marked as a duplicate of this bug. ***

Comment 54

[HARUNAGA Hirotoshi]

*** Bug 240006 has been marked as a duplicate of this bug. ***

Comment 55

[R.K.Aa.]

*** Bug 240045 has been marked as a duplicate of this bug. ***

Comment 56

[R.K.Aa.]

*** Bug 241138 has been marked as a duplicate of this bug. ***

Comment 57

[R.K.Aa.]

*** Bug 241266 has been marked as a duplicate of this bug. ***

Comment 58

[Stefan Borggraefe]

*** Bug 241297 has been marked as a duplicate of this bug. ***

Comment 59

[parv]

(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?