Last Comment Bug 418027 - Win32 buttons do not enter hover state when both focused and hovered (or: hover state should take precedence over focused)
: Win32 buttons do not enter hover state when both focused and hovered (or: hov...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: unspecified
: x86 Windows Vista
: -- trivial (vote)
: mozilla7
Assigned To: Brian R. Bondy [:bbondy]
:
:
Mentors:
: 575480 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-16 19:31 PST by Victor Bielawski
Modified: 2011-07-02 10:55 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
nsNativeThemeWin::StandardGetState was checking for focused and returning early before checking for hover (1.33 KB, patch)
2011-05-27 19:57 PDT, Brian R. Bondy [:bbondy]
jmathies: review+
jmathies: checkin+
Details | Diff | Splinter Review

Description Victor Bielawski 2008-02-16 19:31:10 PST
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.
Comment 1 Jim Mathies [:jimm] 2011-05-24 14:36:04 PDT
*** Bug 575480 has been marked as a duplicate of this bug. ***
Comment 2 Brian R. Bondy [:bbondy] 2011-05-27 19:57:41 PDT
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
Comment 3 Jim Mathies [:jimm] 2011-05-27 21:49:45 PDT
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
Comment 4 Brian R. Bondy [:bbondy] 2011-05-28 12:16:23 PDT
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.
Comment 5 Jim Mathies [:jimm] 2011-06-21 13:21:30 PDT
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.
Comment 6 Jim Mathies [:jimm] 2011-06-29 13:18:16 PDT
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.
Comment 7 Marco Bonardo [::mak] 2011-06-30 05:53:44 PDT
http://hg.mozilla.org/mozilla-central/rev/ffb4d47e4ada

Note You need to log in before you can comment on or make changes to this bug.