New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 636637 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Click on Omnibar icon does not dismissed Translate Bubble on Mac

Project Member Reported by ftang@chromium.org, Aug 11 2016

Issue description

Version: M52 and later
OS: MacOSX only

What steps will reproduce the problem?
(1) visit chrome://flags
(2) locate "New Translate UX Mac" and set to "Enabled", relaunch
(3) visit www.yahoo.co.jp to bring up Translate Bubble.
(4) click on the Omnibar Translate icon on top of the Translate Bubble.

What is the expected output?
Turn off the Translate Bubble 

What do you see instead?
The Bubble flash but stay on.

Please use labels and text to provide additional information.

 

Comment 1 by ftang@chromium.org, Aug 11 2016

I think the issue is in the code 
https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/location_bar/translate_decoration.mm

bool TranslateDecoration::OnMousePressed(NSRect frame, NSPoint location) {
  command_updater_->ExecuteCommand(IDC_TRANSLATE_PAGE);
  return true;
}

The code seems will always run the command command_updater_->ExecuteCommand(IDC_TRANSLATE_PAGE); even if it is already up.

Comment 2 by ftang@chromium.org, Aug 11 2016

We probably should change the code similar to
bool ManagePasswordsDecoration::OnMousePressed(NSRect frame, NSPoint location) {
  if (ManagePasswordsBubbleCocoa::instance())
    ManagePasswordsBubbleCocoa::instance()->Close();
  else
    command_updater_->ExecuteCommand(IDC_MANAGE_PASSWORDS_FOR_PAGE);
  return true;
}
in
https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/location_bar/manage_passwords_decoration.mm

The problem is we need to find out is the TranslateBubbleController active or not but it is currently own by BrowserWindwoController and I have not figure out how to hold on to BrowserWindwoController  from translate_decoration.mm

Comment 3 by ftang@chromium.org, Aug 11 2016

Components: UI>Browser>Translate
I guess I need to use NSNotification to send notification from translate_bubble_controller.mm to  translate_decoration.mm about the translate bubble status.

Comment 4 by groby@chromium.org, Aug 19 2016

We've discussed this elsewhere (sorry, no link handy right now), but:

All you need to do is make sure that a call to Show() will not re-show the bubble if there still is an existing one. (The focus loss of clicking on the icon will automatically close the bubble, but the variable holding the instance will not be reset until after Show is called again)

Comment 5 by ftang@chromium.org, Aug 23 2016

I propose two fixes:
1. https://codereview.chromium.org/2270913002/ (I think this is the right fix)
Looks like groby's earlier attempt to fix crash in bug/609446 cause this side effect.

2. https://codereview.chromium.org/2262913002/ (Please ignore this. This was my early hacky attempt)
Status: Archived (was: Assigned)
Won't be launching the new UI on Mac
Components: -UI>Browser>Translate UI>Browser>Language>Translate

Sign in to add a comment