227713 - General ICON incorrectly highlighted in Tools->Options

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Bill Gianopoulos [:WG9s]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031206 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031206 Firebird/0.7+

Now that the tools->options remembers what options window you looked at last
time instead of always coming up with the General Window, a bug has arisen where
if the remembered window is other than General, the General ICON is incorrectly
colored Light blue.  This is very ugly looking and confusing to users.  I
suggest that this either needs to be fixed for 0.8, or we should go back to
having Tools->Options always open the general Window first.

Reproducible: Always

Steps to Reproduce:
1. Use the default theme
2. Go to Tools->Options 
3. Select any Icon other than General
4. Press cancel to dismiss the Options window
5. Go To Tools->Options a second time
Actual Results:  
The General Icon is incorrectly colored a light-blue.

Expected Results:  
The General Icon background should be white.

Comment 1

[Bill Gianopoulos [:WG9s]]

I forgot to mention that although this appears to be part of the same issue as
bugs 221585, 221687 and 201494, I have purposely opened a seperate bug on this
single aspect of the issue to address what I feel needs to be fixed for 0.8.

Comment 2

[Jason Barnabe (np)]

Confirming and lowering priority. It's really a minor thing.

Comment 3

[José Jeria]

Exaclty how is this bug different from bug 221585?

Comment 4

[Jason Barnabe (np)]

This one is about the General button and the last selected button both being
blue when you open the Options dialog. The problems described in bug 221585 are
different, but both bugs will likely be fixed by the same patch.

Comment 5

[PikeUK]

This is caused by :focus. The code in pref.xul in the second inline script
element (which executes during loading) focuses the same button as the selected
one. However it looks like when the window finishes loading, the focus is reset
to the first control, so the original attempt to set focus is effectively ignored.

A simple fix for this is to move the line button.focus(); from the second inline
script to the last line of the Startup() function in the first inline script
(though I'm a bit dubious about how good an idea it is to define a var in one
block and use it in another, it does work). This is assuming that resetting the
focus is a standard part of window loading, I can't find any code that is
manually resetting focus but I might have missed something.

Comment 6

[Bill Gianopoulos [:WG9s]]

Re comment 3 and comment 4.  No, the idea of this bug was so that they would NOT
both be fixed by a common patch.  There are complicated underlying issues with
fixing bug 221585, which is actually a dupe of bug 201494 and has been around
since dirt.  The reason so many people are seeing the issue now is that the
change in the way the Tool->Options window opens, creates a new, highly visible,
instance of this very old bug.

The purpose of opening this bug is to create a vehicle for removing this highly
visible issue and leaving bugs 221585, 201494 there to work on the underlying
cause.  The intent was to get the incorrect hover highlighting of the general
ICON issue resolved for the 0.8 release becuase it is confusing to users
(especialy those with 256 color monitors or who are partially vision impared so
the selected and hover colors look the same) and it is highly visible.

There are 3 ways to fix this bug without fixing bug 221585 and 201494.

1.  Go back to having Tools->Options always open the General window.
2.  Somehow fix just the Tools->Options so that when it first comes up the
incorect hover highlight is on the selected ICON and not the general ICON.
3.  Remove the hove hightlighiting form the Tools->Options altogether.

Comment 7

[Mike Connor [:mconnor]]

not really a blocker, and bug 201494 and bug 221585 are not the same bug.  part 
of the initial bug report in the second bug covers the first bug, but they're 
not truly duplicate (what I should do is file a new bug explaining all the 
relevant issues and dupe both of those bugs against it to simplify things).

the other bugs are really screwups with how :hover and :focus works, there's 
hacks but they don't fix all other themes so I haven't attached them to any bug.

taking this one, probably will attach a patch based on pike's comments tonight 
(thanks again pike, you rock!)

Comment 8

[Samir L. Boulema]

confirming on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b)
Gecko/20031208 Firebird/0.7+

Comment 9

[PikeUK]

mpconnor are you still intending to attach a patch for this bug? If you want me
to do it I will but I'm not too sure on the best way, i.e. just move
button.focus(); or move the declaration of button up to the first block as well
to make it more readable?

Comment 10

[Mike Connor [:mconnor]]

Attachment: evil

evil is mostly in the mouseover handler that prevents the click+drag
highlighting bug from happening under the default theme.  If there's a better
way of fixing this without a bunch of onmousedown/onmouseup event handlers I'd
be glad to hear it.

I don't think that the button.focus() move is invalid, whether its pretty or
not

Comment 11

[Mike Connor [:mconnor]]

Comment on attachment 137346 [details] [diff] [review]
evil

problem, this breaks opening with arguments (i.e. from the Download Manager)

Comment 12

[Mike Connor [:mconnor]]

Attachment: less evil

plays nice with opening Options to a specific pane (i.e. download manager)

Comment 13

[Bill Gianopoulos [:WG9s]]

Tried the "less evil" patch and it seems to fix all the issues for me!

Comment 14

[Jeff Walden [:Waldo]]

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

Comment 15

[Ben Goodger (use ben at mozilla dot org for email)]

Mike, I've checked this in sans the mouseover focus... not sure why you needed
that... works fine for me without it. 

Comment 16

[Ben Goodger (use ben at mozilla dot org for email)]

Forgot to mark FIXED./ 

Comment 17

[Mike Connor [:mconnor]]

as a note, the mouseover is a cheap hack to fix 201494 (click+drag on one of the
non-highlighted options, you end up with the same effect as this bug).

but, that bug is still open and isn't as important as this one :)

Comment 18

[Bill Gianopoulos [:WG9s]]

I'm not sure what is going on with this Bug.  The comments say the patch was
checked in minus the onmouseover.  Bonsai shows the onmouseover as being present
in the latest version.  Trunk nightlies behave as if the onmouseover is present.
 0.8 branch builds behave as if it is not. ????

Comment 19

[Bill Gianopoulos [:WG9s]]

Please ignore comment #18.  This appears to be intentional.

Comment 20

[Bill Gianopoulos [:WG9s]]

For the record, the onmouseover also fixes the problem of incorrect
hightlighting if you right-click on a non-slected icon.

Comment 21

[Mike Connor [:mconnor]]

Comment on attachment 137347 [details] [diff] [review]
less evil

removing obsolete request

Comment 22

[Mike Connor [:mconnor]]

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

Comment 23

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change