New issue
Advanced search Search tips

Issue 781134 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocked on:
issue 798875

Blocking:
issue 730958



Sign in to add a comment

MacViews: Get Translate Bubble Ready

Project Member Reported by tapted@chromium.org, Nov 3 2017

Issue description

Chrome Version       : 64.0.3251.0
OS Version: OS X 10.12.6

We currently use the infobar on Mac. The Translate bubble seems to need more work before it's shippable. We should keep using the infobar for Harmony phase 1.

There's a test failure

interactive_ui_tests --gtest_filter=AutofillInteractiveTest.AutofillAfterTranslate --enable-features=SecondaryUiMd,ShowAllDialogsWithViewsToolkit

And a partial fix in https://chromium-review.googlesource.com/752783

But it points to more problems.

failure is a sad tab

[42354:775:1103/184613.804484:FATAL:translate_helper.cc(336)] Check failed: false.
5   libchrome_dll.dylib                 0x00000001218ea6df translate::TranslateHelper::RevertTranslation() + 175
6   libchrome_dll.dylib                 0x0000000118f95fa2 translate::mojom::PageStubDispatch::Accept(translate::mojom::Page*, mojo::Message*) + 514
7   libchrome_dll.dylib                 0x00000001218ec143 translate::mojom::PageStub<mojo::RawPtrImplRefTraits<translate::mojom::Page> >::Accept(mojo::Message*) + 83
8   libbindings.dylib                   0x000000010efad922 mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) + 6002
9   libbindings.dylib                   0x000000010efac1a1 mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept(mojo::Message*) + 33
10  libbindings.dylib                   0x000000010efaa2b5 mojo::FilterChain::Accept(mojo::Message*) + 821

bool TranslateHelper::IsTranslateLibAvailable() returns false.


So I don't know how functional this UI is on Mac.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/187e23a1b6de89d7a2b79b4f47c3bfaafa1b59ec

commit 187e23a1b6de89d7a2b79b4f47c3bfaafa1b59ec
Author: Trent Apted <tapted@chromium.org>
Date: Fri Nov 03 21:22:11 2017

Skip AutofillInteractiveTest.AutofillAfterTranslate on MacViews

The test flips ::switches::kEnableTranslateNewUX which is never going
to launch on Mac as Cocoa UI.

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, even with MacViews/Harmony. See  http://crbug.com/781134 . So
only continue when testing the Cocoa UI (i.e. the one that won't launch).

Bug:  780409 ,  781134 
Change-Id: Iaf5aa156f687578e632894f80c9ce6a9b57ccc03
Reviewed-on: https://chromium-review.googlesource.com/752908
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513914}
[modify] https://crrev.com/187e23a1b6de89d7a2b79b4f47c3bfaafa1b59ec/chrome/browser/autofill/autofill_interactive_uitest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/26200f5525047de3095d426af3b6fdf4744f7e49

commit 26200f5525047de3095d426af3b6fdf4744f7e49
Author: Trent Apted <tapted@chromium.org>
Date: Wed Dec 20 00:54:05 2017

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/752783
Reviewed-by: Michael Martis <martis@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525201}
[modify] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/browser/autofill/autofill_interactive_uitest.cc
[rename] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/browser/ui/cocoa/translate/translate_bubble_test_utils_views_cocoa.mm
[modify] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc
[modify] https://crrev.com/26200f5525047de3095d426af3b6fdf4744f7e49/chrome/test/BUILD.gn

Blockedon: 798875
Blocking: 730958
Also note other get-Translate-ready for Mac work in 814477
Status: Fixed (was: Available)
MacViews triage: this happened (done by anthonyvd@) and this work is shipping in M66.

Sign in to add a comment