Commit 26200f55 authored by Trent Apted's avatar Trent Apted Committed by Commit Bot

MacViews: Fix Translate Bubble Anchoring

Currently translate _bubble_ tests on Mac (not the info bar tests)
fail with a DCHECK because the translate bubble does not currently
anchor properly under MacViews. To Anchor properly the bubble needs
a parent window set before calling Widget::Show().

Fix that, and expand test coverage on Mac to include tests for the
bubble.

Note the translate bubble is not enabled by default on Mac (m65 will
continue to use infobars).

Enables on Mac:
 - AutofillInteractiveTest.AutofillAfterTranslate
 - TranslateBubbleViewBrowserTest.* (3 tests)

Bug: 781134, 795987, 507442
Change-Id: I27f8489ad84eb90997ad6f319b178942a07a7a51
Reviewed-on: https://chromium-review.googlesource.com/752783Reviewed-by: default avatarMichael Martis <martis@chromium.org>
Reviewed-by: default avatarMathieu Perreault <mathp@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525201}
parent 16690101
......@@ -1401,13 +1401,6 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_AutofillAfterTranslate) {
#if defined(OS_MACOSX)
if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
::switches::kEnableTranslateNewUX)) {
// On MacViews kEnableTranslateNewUX will engage the MacViews bubble UI
// which isn't ready and isn't used yet. The Cocoa browser still uses an
// InfoBar. See http://crbug.com/781134. So only continue when testing the
// Cocoa UI.
if (base::FeatureList::IsEnabled(features::kShowAllDialogsWithViewsToolkit))
return;
base::CommandLine::ForCurrentProcess()->AppendSwitch(
::switches::kEnableTranslateNewUX);
}
......
......@@ -6,9 +6,11 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/cocoa/browser_dialogs_views_mac.h"
#include "chrome/browser/ui/cocoa/browser_window_controller.h"
#include "chrome/browser/ui/cocoa/translate/translate_bubble_controller.h"
#include "chrome/browser/ui/translate/translate_bubble_model.h"
#include "chrome/browser/ui/views/translate/translate_bubble_view.h"
// TODO(groby): Share with translate_bubble_controller_unittest.mm
@implementation BrowserWindowController (ForTesting)
......@@ -25,6 +27,11 @@ namespace test_utils {
const TranslateBubbleModel* GetCurrentModel(Browser* browser) {
DCHECK(browser);
if (chrome::ShowAllDialogsWithViewsToolkit()) {
TranslateBubbleView* view = TranslateBubbleView::GetCurrentBubble();
return view ? view->model() : nullptr;
}
NSWindow* native_window = browser->window()->GetNativeWindow();
BrowserWindowController* controller =
[BrowserWindowController browserWindowControllerForWindow:native_window];
......@@ -33,6 +40,14 @@ const TranslateBubbleModel* GetCurrentModel(Browser* browser) {
void PressTranslate(Browser* browser) {
DCHECK(browser);
if (chrome::ShowAllDialogsWithViewsToolkit()) {
TranslateBubbleView* bubble = TranslateBubbleView::GetCurrentBubble();
DCHECK(bubble);
bubble->HandleButtonPressed(TranslateBubbleView::BUTTON_ID_TRANSLATE);
return;
}
NSWindow* native_window = browser->window()->GetNativeWindow();
BrowserWindowController* controller =
[BrowserWindowController browserWindowControllerForWindow:native_window];
......
......@@ -14,7 +14,9 @@
#include "base/memory/singleton.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/platform_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/browser/translate/translate_service.h"
......@@ -134,6 +136,17 @@ views::Widget* TranslateBubbleView::ShowBubble(
new TranslateBubbleModelImpl(step, std::move(ui_delegate)));
TranslateBubbleView* view = new TranslateBubbleView(
anchor_view, anchor_point, std::move(model), error_type, web_contents);
#if defined(OS_MACOSX)
// On Mac, there's no anchor view (|anchor_point| is used to position).
// However, the bubble will be set up with no parent and no anchor. That needs
// to be set up before showing the bubble.
DCHECK(!anchor_view);
view->set_arrow(views::BubbleBorder::TOP_RIGHT);
view->set_parent_window(
platform_util::GetViewForWindow(web_contents->GetTopLevelNativeWindow()));
#endif
views::Widget* bubble_widget =
views::BubbleDialogDelegateView::CreateBubble(view);
view->ShowForReason(reason);
......
......@@ -11,6 +11,7 @@
#include "base/macros.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/browser/ui/browser.h"
......@@ -25,6 +26,7 @@
#include "components/translate/core/common/language_detection_details.h"
#include "content/public/browser/notification_details.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/ui_base_features.h"
#include "ui/events/keycodes/dom/dom_code.h"
#include "ui/views/controls/button/menu_button.h"
......@@ -34,7 +36,15 @@ class TranslateBubbleViewBrowserTest : public InProcessBrowserTest {
~TranslateBubbleViewBrowserTest() override {}
void SetUp() override {
#if defined(OS_MACOSX)
// Enable the bubble on Mac (otherwise infobars are used).
base::CommandLine::ForCurrentProcess()->AppendSwitch(
::switches::kEnableTranslateNewUX);
// Enable toolkit-views bubbles on Mac (otherwise Cocoa bubbles are used).
feature_list_.InitAndEnableFeature(features::kSecondaryUiMd);
#else
feature_list_.InitAndDisableFeature(translate::kTranslateUI2016Q2);
#endif
set_open_about_blank_on_browser_launch(true);
translate::TranslateManager::SetIgnoreMissingKeyForTesting(true);
InProcessBrowserTest::SetUp();
......
......@@ -1381,6 +1381,7 @@ test("browser_tests") {
"../browser/ui/views/safe_browsing/password_reuse_modal_warning_dialog_browsertest.cc",
"../browser/ui/views/select_file_dialog_extension_browsertest.cc",
"../browser/ui/views/sync/profile_signin_confirmation_dialog_views_browsertest.cc",
"../browser/ui/views/translate/translate_bubble_view_browsertest.cc",
]
deps += [
"//components/payments/core:test_support",
......@@ -1422,7 +1423,6 @@ test("browser_tests") {
"../browser/ui/views/task_manager_view_browsertest.cc",
"../browser/ui/views/toolbar/browser_actions_container_browsertest.cc",
"../browser/ui/views/translate/translate_bubble_test_utils_views.cc",
"../browser/ui/views/translate/translate_bubble_view_browsertest.cc",
"../browser/ui/views/translate/translate_language_browsertest.cc",
"../browser/ui/views/web_dialog_view_browsertest.cc",
]
......@@ -4791,7 +4791,7 @@ if (is_android) {
sources += [
"../browser/ui/cocoa/extensions/browser_action_button_interactive_uitest.mm",
"../browser/ui/cocoa/permission_bubble/permission_bubble_cocoa_interactive_uitest.mm",
"../browser/ui/cocoa/translate/translate_bubble_test_utils_cocoa.mm",
"../browser/ui/cocoa/translate/translate_bubble_test_utils_views_cocoa.mm",
"base/interactive_test_utils_cocoa.mm",
]
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment