Tab close button correctly highlighted on left click

Tab close button correctly highlighted on left click

BUG=7690

[Peter Kasting, 2009-03-02 17:56:24]

Apparently, I never sent this comment the other day, oops.

http://codereview.chromium.org/28302/diff/1/2
File chrome/browser/views/tabs/tab_renderer.cc (right):

http://codereview.chromium.org/28302/diff/1/2#newcode181
Line 181: BaseButton::OnMousePressed(event);
Here wouldn't you want to return the value returned from
BaseButton::OnMousePressed(), in case it doesn't handle it?

Also, why wouldn't we want to give BaseButton a crack at everything, including
middle clicks?

So, something like:

bool handled = BaseButton::OnMousePressed(event);
// Explicitly mark midle-mouse clicks as non-handled to ensure the tab sees
them.
return event.IsOnlyMiddleMouseButton() ? false : handled;

[Xharze, 2009-03-02 21:40:02]

Changes made according to pkasting comments

[Xharze, 2009-03-09 20:51:21]

What are you comments to the updated patch?

[Peter Kasting, 2009-03-09 20:57:19]

On 2009/03/09 20:51:21, Xharze wrote:
> What are you comments to the updated patch?

It'd be nice to have a comment in the code explaining this slightly confusing
functionality, as I suggested in my last review comment.

[Peter Kasting, 2009-03-09 20:57:42]

Forgot to say, LGTM otherwise, as long as it actually fixes the bug and doesn't
cause other side effects.

[Xharze, 2009-03-09 23:11:19]

Comment added.
During my tests I did not find any side effects. The unit test also runs just
fine. So unless I have missed something the bug are fixed without any side
effects.

[Peter Kasting, 2009-03-10 20:07:33]

Fixed in r11359.

[Mohamed Mansour (USE mhm), 2009-03-10 21:00:59]

http://codereview.chromium.org/28302/diff/2004/3005
File chrome/browser/views/tabs/tab_renderer.cc (right):

http://codereview.chromium.org/28302/diff/2004/3005#newcode189
Line 189: // Explicitly mark midle-mouse clicks as non-handled to ensure the tab
nit: spelling error, middle