Fix mouse wheel redirection when omnibox has focus...

Fix mouse wheel redirection when omnibox has focus

Since focus manager subclass now no longer handlers mouse wheel
redirection, handle WM_MOUSEWHEEL in autocomplete_edit_view_win
and give other windows chance to handle mouse wheel.


Bug 9647



Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=13360

[Peter Kasting, 2009-04-07 18:39:23]

I would prefer to see MSG_WM_MOUSEWHEEL() used instead.

[Peter Kasting, 2009-04-07 19:11:11]

Consistency with surrounding code is more important to me than saving a cast or
macro call inside the function.

[Peter Kasting, 2009-04-07 19:34:36]

http://codereview.chromium.org/63061/diff/1004/6
File chrome/browser/autocomplete/autocomplete_edit_view_win.cc (right):

http://codereview.chromium.org/63061/diff/1004/6#newcode1616
Line 1616: BOOL AutocompleteEditViewWin::OnWheelEvent(UINT flags, short delta,
Nit: three lines for the three params

http://codereview.chromium.org/63061/diff/1004/6#newcode1618
Line 1618: // Forward the mouse-wheel message to the window under the mouse
Nit: Add period at end

http://codereview.chromium.org/63061/diff/1004/6#newcode1621
Line 1621: SetMsgHandled(FALSE);
Nit: Use false instead of FALSE, as surrounding code does

http://codereview.chromium.org/63061/diff/1004/7
File chrome/browser/autocomplete/autocomplete_edit_view_win.h (right):

http://codereview.chromium.org/63061/diff/1004/7#newcode165
Line 165: MSG_WM_MOUSEWHEEL(OnWheelEvent)
Nit: Call this OnMouseWheel (and move function into correct place
alphabetically)

http://codereview.chromium.org/63061/diff/1004/7#newcode251
Line 251: BOOL OnWheelEvent(UINT flags, short delta, CPoint point);
Nit: Should be void return?

[amit, 2009-04-08 16:44:56]

new patch uploaded with changes, please take another look.

http://codereview.chromium.org/63061/diff/1004/6
File chrome/browser/autocomplete/autocomplete_edit_view_win.cc (right):

http://codereview.chromium.org/63061/diff/1004/6#newcode1616
Line 1616: BOOL AutocompleteEditViewWin::OnWheelEvent(UINT flags, short delta,
On 2009/04/07 19:34:36, pkasting wrote:
> Nit: three lines for the three params

Done.

http://codereview.chromium.org/63061/diff/1004/6#newcode1618
Line 1618: // Forward the mouse-wheel message to the window under the mouse
On 2009/04/07 19:34:36, pkasting wrote:
> Nit: Add period at end

Done.

http://codereview.chromium.org/63061/diff/1004/6#newcode1621
Line 1621: SetMsgHandled(FALSE);
On 2009/04/07 19:34:36, pkasting wrote:
> Nit: Use false instead of FALSE, as surrounding code does
Isn't it wrong to pass in a different type and rely that the value of FALSE is
((int) 0)? This is probably academic discussion but if FALSE was defined to
((int) -1) we wouldn't be doing this.

http://codereview.chromium.org/63061/diff/1004/7
File chrome/browser/autocomplete/autocomplete_edit_view_win.h (right):

http://codereview.chromium.org/63061/diff/1004/7#newcode165
Line 165: MSG_WM_MOUSEWHEEL(OnWheelEvent)
On 2009/04/07 19:34:36, pkasting wrote:
> Nit: Call this OnMouseWheel (and move function into correct place
> alphabetically)

Done.

http://codereview.chromium.org/63061/diff/1004/7#newcode251
Line 251: BOOL OnWheelEvent(UINT flags, short delta, CPoint point);
On 2009/04/07 19:34:36, pkasting wrote:
> Nit: Should be void return?

MSG_WM_MOUSEWHEEL macro needs the return value.

[Peter Kasting, 2009-04-08 16:54:04]

On 2009/04/08 16:44:56, amit wrote:
> Line 1621: SetMsgHandled(FALSE);
> On 2009/04/07 19:34:36, pkasting wrote:
> > Nit: Use false instead of FALSE, as surrounding code does
> Isn't it wrong to pass in a different type and rely that the value of FALSE is
> ((int) 0)? This is probably academic discussion but if FALSE was defined to
> ((int) -1) we wouldn't be doing this.

Technically it's not the best, but the specific case of BOOL vs. bool is one
where we elected to use "true/false" consistently through our codebase instead
of "TRUE/FALSE" and so I'd like to stay with that.  I agree that your code is
theoretically better in isolation.

[Peter Kasting, 2009-04-08 16:54:20]

LGTM with nit & FLASE vs. false

http://codereview.chromium.org/63061/diff/1008/10
File chrome/browser/autocomplete/autocomplete_edit_view_win.cc (right):

http://codereview.chromium.org/63061/diff/1008/10#newcode1615
Line 1615: 
Nit: extra newline