420414 - on Vista, a menubutton which has focus and selection should not have a blue background

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P2)

Description

[Mike Beltzner [:beltzner, not reading bugmail]]

Attachment: this is wrong!

Screenshots coming. Looks like we just need to override/remove an explicit background colour statement; transparent background is fine.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Er, that was the screenshot.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yes, very bad!

menulist:focus > .menulist-label-box {
  background-color: transparent ! important;
  color: auto ! important; /* err.. auto? inherit? what's the right thing here? */
}

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #2)
> menulist:focus > .menulist-label-box {
>   background-color: transparent ! important;
>   color: auto ! important; /* err.. auto? inherit? what's the right thing here?
> */
> }

inherit, I think ... at least, for me, auto made the text white when in the selected/collapsed state.

I'll attach a screenshot. The only odd thing left is the focus ring colour, which seems to be tan, not black like everywhere else. I'm tempted to say we should find out where that's being painted and set the colour to be transparent ;)

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Attachment: this is better!

Note the focus ring colour is a little strange ...

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yeah, though I think IE uses the same focus ring color, I think it's kind of a yellow?  If we want something distinct we could set the focus to be the same color as the background was (the blue) maybe..

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

>If we want something distinct we could set the focus to be the same
>color as the background was (the blue) maybe..

Ideally we wouldn't show the focus ring at all, so we really don't want to make it more distinct.

Comment 7

[Dão Gottwald [:dao]]

A focus indicator is needed for accessibility and usability in general.

Comment 8

[Robert Sayre]

looks pretty busted, +

Comment 9

[Ryan A. C.]

Attachment: Dropdown focus ring from the Display dialog in vista

Vista natively uses a black focus ring on dropdowns as the screenshot illustrates. I think black would work fine, it's noticeable enough when tabbing through, and is the usual way of doing things throughout the rest of the OS.

Comment 10

[Alex Faaborg [:faaborg] (Firefox UX)]

>A focus indicator is needed for accessibility and usability in general.

Yeah, what I meant to say was that ideally we wouldn't be showing the focus ring on mouse interactions (bug 418521).

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

While I agree with bug 418521, let's not make a hack fix for that just for a single XUL element. I never should have joked. Vlad: same colour as everywhere else, please, which seems to be black.

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #11)
> While I agree with bug 418521, let's not make a hack fix for that just for a
> single XUL element. I never should have joked. Vlad: same colour as everywhere
> else, please, which seems to be black.

You're asking the wrong person here, this is a theme css thing :)

Comment 13

[Luke Iliffe (Harlequin99)]

Not sure if this is the same thing but on vista when dragging a favicon over a folder in the bookmark menu there is dark blue selection highlighting as well as the lighter blue vista selection. Also the selection seems to remain on the top 'bookmark this page' menu entry as well as being duplicated over the drop folder target. 

Screenshot following.

Comment 14

[Luke Iliffe (Harlequin99)]

Attachment: darkblue selection plus duplicate selection.

Comment 15

[Ryan A. C.]

Does the resolution to Bug 418537 effect this in any way?

Comment 16

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Nope, doesn't affect this.

Comment 17

[mcdavis941 (sporadically reading bugmail)]

I can't take Vista-specific bugs at the moment.  Sorry.  Setting back to unassigned.

Comment 18

[(no longer active)]

Attachment: Patch (v1)

Fix the problem on the aero theme.

Comment 19

[Ryan A. C.]

The 1px dotted border should probably change color too. We could probably get away with setting it to the same color as the text in the box, that way it'll almost always be black, but also show up in the same contrast as the text itself on any other theme.

Comment 20

[(no longer active)]

Attachment: Patch (v1.1)

(In reply to comment #19)
> The 1px dotted border should probably change color too. We could probably get
> away with setting it to the same color as the text in the box, that way it'll
> almost always be black, but also show up in the same contrast as the text
> itself on any other theme.

This new patch draws the focus ring using ThreeDDarkShadow, which is the same color as the focus ring for a XUL button.

Comment 21

[Mike Connor [:mconnor]]

Comment on attachment 313888 [details] [diff] [review]
Patch (v1.1)

r+a=mconnor, thanks!

Comment 22

[:Gavin Sharp [email: [e-mail]]]

mozilla/toolkit/themes/winstripe/global/jar.mn 	1.50
mozilla/toolkit/themes/winstripe/global/menulist-aero.css 	1.1