Translate: Have the bubble not to steal focus

Translate: Have the bubble not to steal focus

This CL changes the Translate bubble's behavior not to steal the focus.

BUG=378643
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288019

[hajimehoshi, 2014-07-31 05:57:34]

PTAL

[sky, 2014-07-31 15:53:19]

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1044:
TranslateBubbleView::CloseBubble();
Why is keying off LocationBarView::UPdate the right place to close the bubble?

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/translate/translate_bubble_view.cc (right):

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/translate/translate_bubble_view.cc:140:
translate_bubble_view_->GetWidget()->CloseNow();
Is there a reason you need closenow rather than close?

[hajimehoshi, 2014-08-01 09:46:06]

Thank you!

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1044:
TranslateBubbleView::CloseBubble();
On 2014/07/31 15:53:19, sky wrote:
> Why is keying off LocationBarView::UPdate the right place to close the bubble?

It's because ZoomBubbleView seems to do the same thing (this is just my guess
from the code). I want the bubble dismiss when the activated tab is switched,
and LocationBarView::Update seemed the right place. Is there better place?

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/translate/translate_bubble_view.cc (right):

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/translate/translate_bubble_view.cc:140:
translate_bubble_view_->GetWidget()->CloseNow();
On 2014/07/31 15:53:19, sky wrote:
> Is there a reason you need closenow rather than close?

No. Done.

[sky, 2014-08-04 18:44:26]

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/425223003/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1044:
TranslateBubbleView::CloseBubble();
On 2014/08/01 09:46:06, hajimehoshi wrote:
> On 2014/07/31 15:53:19, sky wrote:
> > Why is keying off LocationBarView::UPdate the right place to close the
bubble?
> 
> It's because ZoomBubbleView seems to do the same thing (this is just my guess
> from the code). I want the bubble dismiss when the activated tab is switched,
> and LocationBarView::Update seemed the right place. Is there better place?

I think you should listen for the tabstrip selection to change and not assume
the two are the same. Adding Peter to see if he agrees.

[Peter Kasting, 2014-08-04 18:51:26]

Update() will be hit both when the current page navigates and when the active
tab changes.  For Zoom, we want the bubble to close in both these cases, so
keying off Update() is correct.  Whether it's correct for you depends on whether
you also want both these behaviors.

If you want to test, consider testing with-page anchor navigations and the like
to see if (a) they close the bubble and (b) that's actually what you want.

[hajimehoshi, 2014-08-06 04:54:54]

Thank you for the reply!

On 2014/08/04 18:51:26, Peter Kasting wrote:
> Update() will be hit both when the current page navigates and when the active
> tab changes.  For Zoom, we want the bubble to close in both these cases, so
> keying off Update() is correct.  Whether it's correct for you depends on
whether
> you also want both these behaviors.

The cases when closing the translate bubble are the same as those of zoom.
Update() seems to work.

[sky, 2014-08-06 17:39:10]

LGTM

[hajimehoshi, 2014-08-07 04:11:41]

The CQ bit was checked by hajimehoshi@chromium.org

[commit-bot: I haz the power, 2014-08-07 04:13:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hajimehoshi@chromium.org/425223003/40001

[commit-bot: I haz the power, 2014-08-07 08:17:35]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-07 09:53:26]

Change committed as 288019