Translate popup shows outside the DPWA window
Reported by
henry71...@gmail.com,
Dec 14
|
|||
Issue descriptionUserAgent: 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:
,
Dec 14
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).
,
Jan 9
Strange, it appears inside the window when I try. Using Mac 10.14.2 Chrome canary 73.0.3665.0.
,
Jan 11
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
,
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
,
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
,
Jan 16
Confirmed this fix works in canary. |
|||
►
Sign in to add a comment |
|||
Comment 1 by mathias@chromium.org
, Dec 14Labels: OS-All
Status: Untriaged (was: Unconfirmed)