430434 - dropdown button on menulists in Vista chrome should not have a hover state

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

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

Menulist buttons on Vista don't have a hover state for the dropdown button, and instead have a hover state for the entire button. We're inconsistent, and the existing hover state doesn't align properly to the edges of the menulist button.

(Note: this is chrome only, we're fine in content)

Comment 1

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

Attachment: always assume normal state if vista & not-content

Pretty sure that this is right, not at all sure how to test it (litmus?). I made sure that it didn't affect content area menulists, and has the desired effect.

I'm sure I messed up some coding conventions :)

Comment 2

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

Attachment: before and after

Comment 3

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

Attachment: example of menulist in Vista

Comment 4

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

(the fact that our default button size is larger is, btw, a separate bug, and next on my "what to do when I'm bored and poking around in nsThemeWin" list)

Comment 5

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

Attachment: always assume norma state if vista & not content (v2)

Now with improved whitespace management (thx, johnath!)

Comment 6

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

Comment on attachment 317307 [details] [diff] [review]
always assume norma state if vista & not content (v2)

r+a=me with grammar fixes as discussed!

Comment 7

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

Attachment: always assume normal state if vista & not content (v3)

Grammar & spacing nits fixed, carrying over r+a=vladimir

Comment 8

[Reed Loden [:reed]]

Checking in widget/src/windows/nsNativeThemeWin.cpp;
/cvsroot/mozilla/widget/src/windows/nsNativeThemeWin.cpp,v  <--  nsNativeThemeWin.cpp
new revision: 3.121; previous revision: 3.120
done

Comment 10

[Marcia Knous [:marcia]]

This looks ok to me using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008042407 Minefield/3.0pre - the only thing I notice is that the focus ring hangs around after selection, but this happens consistently in Options in any other field like that one.

Comment 11

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008042407 Minefield/3.0pre. I will file a spinoff bug on Comment 10.

Comment 12

[Brian R. Bondy [:bbondy]]

Attachment: Editable menulists were not using -moz-appearance: menulist-textfield;

I was debugging nsNativeThemeWin but NS_THEME_DROPDOWN_TEXTFIELD was never called when drawing the widget background.

The problem was that the menulist's -moz-appearance was being inherited.
The fix was to set a different moz-appearance: 

menulist[editable="true"] {
  -moz-appearance: menulist-textfield;
}

After this fix, NS_THEME_DROPDOWN_TEXTFIELD gets called correctly when drawing the widget background mentioned above.

I also checked to see if this problem was on OS X, Ubuntu, and Kubuntu, but everything worked normal on those with default settings and no code changes.
I also verified in the other menulist.css files for other platforms and they all have similar settings that I came up with, winstripe was the only one missing it.

On a side note, you may notice the dropdown button does not receive focus as it should, this is because of the following task and not related to this bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=430434

Comment 13

[Jim Mathies [:jimm]]

Comment on attachment 536213 [details] [diff] [review]
Editable menulists were not using -moz-appearance: menulist-textfield;

I'm not a peer here, Dao should review this.

For reference: https://wiki.mozilla.org/Modules

Comment 14

[Jim Mathies [:jimm]]

Also, Brian, this bug is resolved verified, so you should open up a new bug and mark it as dependent (or blocking if this fixes a regression) instead of posting the patch on a bug that is already closed out.

Comment 15

[Dão Gottwald [:dao]]

(In reply to comment #14)
> Also, Brian, this bug is resolved verified, so you should open up a new bug
> and mark it as dependent (or blocking if this fixes a regression) instead of
> posting the patch on a bug that is already closed out.

Yep.

(In reply to comment #12)
> On a side note, you may notice the dropdown button does not receive focus as
> it should, this is because of the following task and not related to this bug:
> https://bugzilla.mozilla.org/show_bug.cgi?id=430434

430434 is this bug.

Comment 16

[Brian R. Bondy [:bbondy]]

Sorry Jim and Dao, Comment 12 and its patch was meant for Bug 431078.  I will repost there.