84879 - Crash when using color picker in preference menu

Status: RESOLVED DUPLICATE of bug 160656

(Core :: XUL, defect)

Description

[doctor__j]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.1+) Gecko/20010607
BuildID:    2001060809

Reproducible: Always
Steps to Reproduce:
1. Open Preference menu
2. Go to "Appearance -> Colors"
3. Open the color picker to choose a color

Actual Results:  Pref menu title bar flashes rapidly and then crash occured.

Crash message from Windows:
MOZILLA caused a stack fault in module KERNEL32.DLL at 017f:bff7429f.

Talkback failed to start.  I can't submit a talkback crash info.

Any relations to bug 64160?

Comment 1

[Matthias Versen [:Matti]]

wfm with win2k build 20010609.. (CVS opt)

Comment 2

[Cormac F]

Im seeing this on win98 build:2001060820
Looks like an infinite loop error of some sort, if I was to guess

Comment 3

[Blake Ross]

--> cmanske
Not seeing in win2k, must be 9x-specific.  I'll bet the cause here is similar 
to bug 63452.

Comment 4

[Blake Ross]

Toolkit, actually.

Comment 5

[Charles Manske]

wfm on NT4.0 with both Classic and Modern Skin.
My changes to colorpicker for bug 64160 were carefully crafted to not affect
the existing colorpicker. The only change that would reawaken a problem (that
does very much seem like bug 63452) would be the addition of:
colorpicker {
  -moz-user-focus: normal;
}
to all colorpicker.css files.
But why does this only occur on Win9x !!!!
What about other platforms? Please be sure to test both Classic and Modern
themes, as (unfortunately), there are subtle differences in focus handling
in each.
Suggested interim fix would be to remove "-moz-user-focus: normal;"
from colorpicker.css and add it only to Composer's CSS, because Composer is
the only user of "colorpicker" without the "colorpicker-button". Thus only
Composer needs the "normal" focus (without it, user can't tab to/from the
colorpicker swatches in Composer's colorpicker dialog.)

Comment 6

[Charles Manske]

Attachment: Possible fix?

Comment 7

[doctor__j]

I can reproduce the crash using both Classic and Modern themes.
BTW, talkback can't catch the crash... I can't even start the Talkback agent.
<sigh>

Comment 8

[Charles Manske]

Attachment: Better suggested fix: restrict "-moz-user-focus: normal" to just Composer's colorpicker dialog

Comment 9

[Charles Manske]

Since I'm only running WinNT 4.0, I can't test this bug at all, so can someone
please try the attachment "Better suggested fix?".
Please test with both Classic and Modern themes, and test if Prefs -> Appearance
-> color buttons work in both Browser and Composer.
Also: Does anyone see any problems using Composer's colorpicker (with or without
the new patch) under Win9x?

Comment 10

[Blake Ross]

Hrm...I'm not sure I understand why this would fix it (I can't test either, 
though).  Wasn't bug 63452 fixed by *allowing* the colorpicker to take focus, so 
the keystrokes went to it and not the pref tree, so the user had to dismiss the 
colorpicker before switching panes?

Comment 11

[Charles Manske]

I have no idea if my suggested patch will fix this, but it gets us to very close
to the state before my changes for bug 64160, so I'm trying to eliminate my
changes as the cause.

Comment 12

[rubydoo123]

I can't reproduce this on win98 using the opt build from 6/11, asking QA to 
verify

Comment 13

[Chad Austin]

no can repro in Win95 (p133, 48 MB of RAM), build 20010608.

tried both classic and modern

Comment 14

[sairuh (rarely reading bugmail)]

yes, i can repro this using 2001.06.11.09 comm trunk bits on a win32 machine in
the testlab. i see the same flashing effect as dr_j originally reported, then crash.

the test machine was running Win Me v4.90.3000; it's agonizingly slow [only 32M
RAM].

unfortunately, the stack trace is paltry --no symbols :(

Incident ID 31595372 
 Stack Signature
                KERNEL32.DLL + 0x4277 (0xbff64277) c243d2bd 
 Bug ID
 Trigger Time                2001-06-11 15:48:16 
 User Comments                win Me, 4.90.3000: choose color from picker 
 Build ID                2001061109 
 Product ID                MozillaTrunk 
 Platform ID                Win32 
 Stack Trace

KERNEL32.DLL + 0x4277 (0xbff64277) 
0x07200720 

Comment 15

[Charles Manske]

But can anyone reproduce this after applying my 6/10/01 09:20 patch?

Comment 16

[sairuh (rarely reading bugmail)]

blake, i cannot remember if you have access to WinMe, much less a build system
on it?

beppe, out of curiosity --was your 6/11 opt build from the branch or trunk?

Comment 17

[John Morrison]

We can just hand-roll cmanske's patch into a release build and then try 
to reproduce this on the machine where you could crash, sairuh. So, to 
quote an obscure song "I'll do the rolling, you do the details."

Comment 18

[Charles Manske]

John's suggestion is good. Basically, my patch is totally safe and can't hurt!
And if we're lucky, it just might help!
Anyone for r=, sr=, please?

Comment 19

[sairuh (rarely reading bugmail)]

jrgm added your changes to the 2001.06.11.09 trunk build on the WinMe machine
--and the color picker worked just dandy [both modern and classic, as well as
both the prefs and editor color picker].

Comment 20

[Charles Manske]

Super! Then let's get this approved and checked in.

Comment 21

[Simon Fraser [no longer active]]

Why are we patching up a crash with CSS changes? It sounds like this is some kind 
of focus-related infinite loop (cc'ing saari). Can anyone get a stack?

Comment 22

[Kathleen :Brade]

I think Charley's fixes definitely should be checked in since they are supposed 
to be for Composer-only color pickers (not global).

However, I am afraid that we are simply masking the problem.  I'd like someone to 
generate a stack crawl or point the finger to some specific code that is causing 
this problem before this fix goes in (or put the fix only on a branch).

conditional r=brade

Comment 23

[Charles Manske]

I suggest we checkin the additions of

#ColorPicker {
  -moz-user-focus: normal;
}

to the Editor CSS files as in the patch.
This will assure that the colorpicker focus is correct for editor, but will not
actually fix this bug. It will simply be a redundant rule.
We then reassign (to Saari?) to investigate the real problem. If real solution
can't be found before RTM, then it is easier to just remove the
"-moz-user-focus: normal;" from the global CSS files as an interim fix.
It is very important that the color buttons not crash in the Browser!

Comment 24

[Kathleen :Brade]

purify on my NT build doesn't show anything unusual...  :-(

Comment 25

[John Morrison]

Note that this crash has only been seen on 9x systems (98/ME). I just tried to 
get a stack trace on a 98 build, but vc++ would not come up (at least on that 
machine).

Comment 26

[Charles Manske]

Attachment: Use this to remove "-moz-user-focus: normal" from global CSS. MUST CHECK THIS IN BEFORE RTM IF NO OTHER FIX IS FOUND!

Comment 27

[Charles Manske]

The editor CSS now has the required "-moz-user-focus: normal" for its own
colorpicker. This bug is being reassigned to the menu/event expert for further
investigation. But it no fix is found, the patch on 06/13/01 14:47 MUST BE
CHECKED IN to prevent the crash on Win9x platforms.
If we do have to use that patch, this bug should be kept open for future
investigation. To restore back to the "bug" condition, simply add back:
colorpicker {
  -moz-user-focus: normal;
}

to colorpicker.css in themes/modern/global, and themes/classic/global/win.
brade has supplied the "conditional" r= for this patch.

Comment 28

[Joe Hewitt (gone)]

a sad sad sr=hewitt

Comment 29

[Christopher Blizzard (:blizzard)]

A sad, sad a=blizzard on behalf of drivers for the trunk. :(

Comment 30

[Kathleen :Brade]

work around checked in for RTM; moving to mozilla 1.0

reminder: to reproduce this bug you'll need to back out the attachment from 
6/13/01 14:47.

Comment 31

[doctor__j]

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

Comment 32

[AriB]

wfm win98 build 2001063008

Comment 33

[Cormac F]

This seems to be fixed now (build:2001070708, win98)

Comment 34

[John Morrison]

May God have mercy on us all. The 212 bug spam-o-rama is Now!

Comment 35

[Joe Hewitt (gone)]

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

Comment 36

[sairuh (rarely reading bugmail)]

is this still a problem on win98?

this is not a problem for me on winNT using 2001.11.20.09-comm bits, using the
modern theme. marking wfm, but do reopen if still a problem.

Comment 37

[Julien Cegarra]

Worksforme (Build ID: 2002 01 11 03/Win98).

Could we close that one now ? 

/jc

Comment 38

[doctor__j]

Marking so...

Comment 39

[Daniel Wang]

dupe of bug 160656

Comment 40

[Daniel Wang]

*** This bug has been marked as a duplicate of 160656 ***

Comment 41

[Kathleen :Brade]

I disagree with this bug being a duplicate for several reasons.  If it was going
to be a duplicate, it should've been duplicated to bug 156486 (not a bug that
itself is a duplicate).  Both of those bugs were regressions that were filed
well after this bug was filed and resolved as worksforme.