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

Issue 915169 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Translate popup shows outside the DPWA window

Reported by henry71...@gmail.com, Dec 14

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3639.0 Safari/537.36

Steps to reproduce the problem:
1. Enable #enable-desktop-pwas flags
2. Open any PWA in Desktop PWA mode (make sure the site is not your default language, so the translate popup will appear) 
3. The translate popup shows outside the app window

What is the expected behavior?

What went wrong?
Translate popup shows outside the Desktop Progressive Web App (DPWA) window

Did this work before? N/A 

Chrome version: 73.0.3639.0  Channel: n/a
OS Version: OS X 10.14.0
Flash Version:
 
Screenshot 2018-12-14 at 4.40.25 PM.png
620 KB View Download
Components: UI>Browser>WebAppInstalls
Labels: OS-All
Status: Untriaged (was: Unconfirmed)
Cc: sdy@chromium.org ccameron@chromium.org
Components: -UI
Labels: -OS-All M-73
Owner: alancutter@chromium.org
Status: Assigned (was: Untriaged)
Looks like a views dialog to me. This could be a Mac specific anchoring issue, or a general anchoring issues for all translate bubbles (though I'd find it weird that it hasn't been reported yet if it's a general issue since we've been launched on non Mac desktop for a couple of milestones now).
Strange, it appears inside the window when I try.
Using Mac 10.14.2 Chrome canary 73.0.3665.0.
Screen Shot 2019-01-09 at 3.46.13 pm.png
424 KB View Download
This should be properly fixed by adding the translate icon to the PWA title bar and anchoring the bubble to the app menu button.

Screenshots of WIP CL: https://chromium-review.googlesource.com/c/chromium/src/+/1404764

before.png
151 KB View Download
after.png
220 KB View Download
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 14

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

commit a3e848b7ef33e28bf47d64cb29842220e5bf0482
Author: Alan Cutter <alancutter@chromium.org>
Date: Mon Jan 14 07:29:48 2019

Add translate icon to PWA title bars

This CL migrates the TranslateIconView to live inside
PageActionIconContainerView so that it may be added to PWA/hosted app
title bars.

This fixes translate bubble anchoring for PWA windows and enables
the translate button so the user can access translate options for
the page at any time.

Note: In HostedAppButtonContainer UpdateContentSettingsViewsVisibility()
was changed to UpdateStatusIconsVisibility() to include the
PageActionIconViews as well. This ensures the translate button is shown
for translatable pages even if Chrome decides not to show the translate
prompt.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=374637&signed_aid=uRZeXk92KtwW4_PtdcAOEQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=374638&signed_aid=3ic5030oiWCC7ryBkJUt_Q==&inline=1

Bug:  915169 
Change-Id: Iaf7eb6a0ef653b0441bed924be0931b6274f2ff7
Reviewed-on: https://chromium-review.googlesource.com/c/1404764
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622392}
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/page_action/page_action_icon_container.h
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/frame/hosted_app_button_container.cc
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/frame/hosted_app_button_container.h
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/location_bar/location_icon_view_interactive_uitest.cc
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/page_action/page_action_icon_container_view.cc
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/page_action/page_action_icon_container_view.h
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/toolbar/toolbar_view.cc
[modify] https://crrev.com/a3e848b7ef33e28bf47d64cb29842220e5bf0482/chrome/browser/ui/views/toolbar/toolbar_view.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 15

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

commit 94f8376ac902a1140a54e6213e7f16c13e6a2eae
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue Jan 15 03:10:22 2019

Clean up unused anchor_point parameter in TranslateBubbleView

The only callsite of TranslateBubbleView::ShowBubble sets anchor_point
to gfx::Point(). We can remove this parameter entirely.

Bug:  915169 
Change-Id: I3a5c7aec68288d18c327ce1740d5694abf523576
Reviewed-on: https://chromium-review.googlesource.com/c/1408691
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622703}
[modify] https://crrev.com/94f8376ac902a1140a54e6213e7f16c13e6a2eae/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/94f8376ac902a1140a54e6213e7f16c13e6a2eae/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/94f8376ac902a1140a54e6213e7f16c13e6a2eae/chrome/browser/ui/views/translate/translate_bubble_view.h
[modify] https://crrev.com/94f8376ac902a1140a54e6213e7f16c13e6a2eae/chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc

Status: Fixed (was: Assigned)
Confirmed this fix works in canary.
Screen Shot 2019-01-16 at 2.46.04 pm.png
381 KB View Download

Sign in to add a comment