Win32 buttons do not enter hover state when both focused and hovered (or: hover state should take precedence over focused)

RESOLVED FIXED in mozilla7

Status

()

Core
Widget: Win32
--
trivial
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: Victor Bielawski, Assigned: bbondy)

Tracking

unspecified
mozilla7
x86
Windows Vista
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b4pre) Gecko/2008021504 Minefield/3.0b4pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b4pre) Gecko/2008021504 Minefield/3.0b4pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021504 Minefield/3.0b4pre

There is nothing to add to the summary except a screenshot: http://img528.imageshack.us/img528/5906/win32buttonfocushoverbuph5.png

Reproducible: Always

Steps to Reproduce:
1. Focus a button
2. Hover the mouse over it.

Actual Results:  
The button is in the focused state.

Expected Results:  
The button should be in the hover state.

This bug requires visual styles to be turned on.

Updated

6 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

6 years ago
Duplicate of this bug: 575480
(Assignee)

Updated

6 years ago
Assignee: nobody → netzen
(Assignee)

Comment 2

6 years ago
Created attachment 535808 [details] [diff] [review]
nsNativeThemeWin::StandardGetState was checking for focused and returning early before checking for hover

When a button is focused, and mouse hovers over, it will now show the hovered state and focus rect.  

Please let me know if there should be a test case defined for this, and if so any hints of similar bugs with similar tests would be appreciated.

I ran this patch through the following automated and there were no fails, but it ran very quickly, please let me know if there are other tests I should be running as well.  
I ran:
TEST_PATH=widget/tests/ make -C $(OBJDIR) mochitest-plain
Attachment #535808 - Flags: review?(jmathies)

Comment 3

6 years ago
Comment on attachment 535808 [details] [diff] [review]
nsNativeThemeWin::StandardGetState was checking for focused and returning early before checking for hover

Review of attachment 535808 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/src/windows/nsNativeThemeWin.cpp
@@ +510,5 @@
>  
>  PRInt32
>  nsNativeThemeWin::StandardGetState(nsIFrame* aFrame, PRUint8 aWidgetType,
>                                     PRBool wantFocused)
>  {

This will apply to a number of ui elements, not just buttons. Are you sure this is the appropriate default behavior for everything this applies to?

http://mxr.mozilla.org/mozilla-central/search?string=StandardGetState
(Assignee)

Comment 4

6 years ago
Upon further inspection, fix looks good and also fixes another problem (see NS_THEME_TAB).
I mention below the before and after effect for all of the widget types that use the modified function:

When focused and the pointer is on top:

NS_THEME_BUTTON
- Before shows focus outline but no highlight color for mouse pointer (original reported problem)
- After Shows both focus outline and highlight color

NS_THEME_CHECKBOX
NS_THEME_RADIO
- Before shows focus outline and highlight color when pointer goes over
- After works same

NS_THEME_TEXTFIELD
NS_THEME_TEXTFIELD_MULTILINE
- Before shows slight border change with pointer on top, slightly darker change with focus
- After works same 

NS_THEME_SPINNER_UP_BUTTON
NS_THEME_SPINNER_DOWN_BUTTON
- Cannot focus, gets highlight color (no focus even with -moz-user-focus: normal;)
- After works same

NS_THEME_TAB
- Before shows focus only on the selected tab, other tabs show highlight color as mouse pointer goes over. 
  If I force focus on non active tabs with tab { -moz-user-focus: normal } though it has the same bug as the original reported bug for button
- After shows same if no explicit focus is specified.  If -moz-user-focus:normal on non active tabs, fixes problem as with NS_THEME_BUTTON

NS_THEME_TREEVIEW_HEADER_CELL
- Before does not get focus, does get highlight color
  -moz-user-focus:normal has no effect
- After works same

On a side note, should the spinner not being able to be focused with "-moz-user-focus: normal;" be posted?  Please advise and I can attempt to post if it is not already posted.
(Assignee)

Updated

6 years ago
Attachment #535808 - Flags: review?(jmathies) → review?(dao)

Updated

6 years ago
Attachment #535808 - Flags: review?(dao) → review?(jmathies)

Comment 5

6 years ago
Comment on attachment 535808 [details] [diff] [review]
nsNativeThemeWin::StandardGetState was checking for focused and returning early before checking for hover

Thanks for the detailed write up! I tested each of these and compared them to IE, everything looks ok.
Attachment #535808 - Flags: review?(jmathies) → review+
(Assignee)

Updated

6 years ago
Attachment #535808 - Flags: checkin?(jmathies)

Comment 6

6 years ago
Comment on attachment 535808 [details] [diff] [review]
nsNativeThemeWin::StandardGetState was checking for focused and returning early before checking for hover

http://hg.mozilla.org/integration/mozilla-inbound/rev/ffb4d47e4ada

I think the rule on this new flag is to just set checkin? w/o an email. People who land stuff will come around and find it. Note this bug should remain open until the patch merges to mc.
Attachment #535808 - Flags: checkin?(jmathies) → checkin+
http://hg.mozilla.org/mozilla-central/rev/ffb4d47e4ada
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.