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

Issue 877032 link

Starred by 2 users
Status: Verified
Owner:
alancutter@chromium.org
Closed: Sep 13
Cc:
vamshi.kommuri@chromium.org
█ mgiuca@chromium.org
nyerramilli@chromium.org
robliao@chromium.org
kkaluri@chromium.org
ligim...@chromium.org
dominickn@chromium.org
█ bgalbs@google.com
pcovell@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug



Sign in to add a comment

Desktop PWAs: Permission/App Info bubble clipped by app window

Project Member Reported by alancutter@chromium.org, Aug 23 
Chrome Version: 70
OS: Windows 7 (non-aero)

Bubble dialogs are clipped to the hosted app window on Windows 7.
This is particularly bad for PWAs as they are anchored on the right side and always get clipped making the bubble unusable.

See screenshot.
permission-clipped.png
12.1 KB View Download
app-info-clipped.png
24.3 KB View Download
expected.png
46.1 KB View Download

Comment 1 by alancutter@chromium.org, Aug 27 

This is what it looks like if you force the dialog to appear in its own desktop native widget without Aero enabled on Windows 7.
w7notransparency.png
16.9 KB View Download

Comment 2 by alancutter@chromium.org, Aug 27 

This was done by messing with ChromeViewsDelegate::CreateNativeWidget() in chrome_views_delegate_win.cc.

Comment 3 by groby@google.com, Aug 27

Cc: robliao@chromium.org
+robliao as fyi

Comment 4 by alancutter@chromium.org, Aug 29 

Current plan is to show these dialogs using a simplified border (no shadow) when the OS doesn't support shadows.
degraded-bubble.png
13.8 KB View Download

Comment 5 by alancutter@chromium.org, Aug 29 

Currently the NO_SHADOW shadow type doesn't disable shadows.
WIP screenshots of making NO_SHADOW not render a shadow:
before.png
22.7 KB View Download
after.png
22.0 KB View Download

Comment 6 by alancutter@chromium.org, Aug 29 

WIP screenshots of clipping fix.
before.png
17.2 KB View Download
after.png
21.1 KB View Download

Comment 7 by alancutter@chromium.org, Aug 30 

This problem also exists for Linux which we hard code disabled bubbles from appearing outside the main widget.
Screenshots from WIP CL to change this on Ubuntu 16 (the earliest Linux version we run try bots for):
ubuntu16before.png
176 KB View Download
ubuntu16after.png
209 KB View Download

Comment 8 by alancutter@chromium.org, Aug 30

Labels: OS-Linux
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 30 

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

commit ef6fd66e935607f7e58d9f05c49121dc2d7e7788
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu Aug 30 21:52:43 2018

Simplify BubbleBorderTest.GetBoundsOriginTest

Bubble widgets no longer have a visible arrow, this simplifies
the bounds layout calculation.
This CL removes redundant constants that are either 0 or equal
to other constants in BubbleBorderTest.GetBoundsOriginTest.

Bug:  877032 
Change-Id: Icb886704647f926acbf29c23df0695ea36cd9b5b
Reviewed-on: https://chromium-review.googlesource.com/1195244
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587797}
[modify] https://crrev.com/ef6fd66e935607f7e58d9f05c49121dc2d7e7788/ui/views/bubble/bubble_border_unittest.cc

Comment 10 by alancutter@chromium.org, Aug 31 

My mistake, we run try bots with Ubuntu 14. Good news is our fancy bubbles are compatible with that OS too.
ubuntu14bubbles.png
288 KB View Download

Comment 11 by mgiuca@chromium.org, Aug 31

Cc: nyerramilli@chromium.org kkaluri@chromium.org mgiuca@chromium.org ligim...@chromium.org
 Issue 879468  has been merged into this issue.

Comment 12 by alancutter@chromium.org, Sep 3 

Even with bubbles appearing outside of the main widget we don't allow them to rearrange themselves on all platforms resulting in them going off the screen for maximised windows. We should really be anchoring the top right of the bubble for things anchoring off the app menu buttons.

Screenshots of WIP CL: https://chromium-review.googlesource.com/c/chromium/src/+/1201507
before.png
170 KB View Download
after.png
177 KB View Download

Comment 13 by vamshi.kommuri@chromium.org, Sep 3

Cc: vamshi.kommuri@chromium.org
Labels: Needs-Feedback
Tried checking the issue using latest chrome version 71.0.3541.0 using Ubuntu 14.04. We observed the info bubble is still clipped by app window. Attaching the screen shot for reference.

@Alan Cutter: Please let us know if any fixes are yet to land here.

Thanks!
877032 CL Verification.png
147 KB View Download

Comment 14 by alancutter@chromium.org, Sep 4

Labels: -Needs-Feedback
#13:
There are multiple attempts at fixing this in flight.
The current attempt I hope to land and merge to M70 is: https://chromium-review.googlesource.com/c/chromium/src/+/1201507
Project Member

Comment 15 by bugdroid1@chromium.org, Sep 7 

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

commit fa02ae6a0b4d0ace080f3eb32febc768dac59670
Author: Alan Cutter <alancutter@chromium.org>
Date: Fri Sep 07 08:02:29 2018

Make page info and permission bubbles anchor from the top right in hosted app windows

This CL fixes a bug where permission prompts and page info dialogs go off screen when
hosted app windows are maximised. This only affects platforms that don't support bubble
anchor adjustment (see calls to set_adjust_if_offscreen(false) in
BubbleDialogDelegateView::CreateBubble()). These platforms are Linux, Mac and Windows 7
with Aero disabled.

This patch fixes the issue by making permission and page info dialogs anchored off
the app menu use TOP_RIGHT instead of TOP_LEFT for their arrow position leading the
bubble to position itself within the main window's bounds by default.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355950&signed_aid=ziHkOFUJALIjs_7uaUaPtQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355951&signed_aid=9B2p0GLHiuHOMC-Ha-rXBQ==&inline=1

Bug:  877032 
Change-Id: I98b1fe1e4fe8018886ae960c8601f5d518c9ea5e
Reviewed-on: https://chromium-review.googlesource.com/1201507
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589455}
[modify] https://crrev.com/fa02ae6a0b4d0ace080f3eb32febc768dac59670/chrome/browser/ui/cocoa/browser_dialogs_views_mac.cc
[modify] https://crrev.com/fa02ae6a0b4d0ace080f3eb32febc768dac59670/chrome/browser/ui/views/bubble_anchor_util_views.cc
[modify] https://crrev.com/fa02ae6a0b4d0ace080f3eb32febc768dac59670/chrome/browser/ui/views/bubble_anchor_util_views.h
[modify] https://crrev.com/fa02ae6a0b4d0ace080f3eb32febc768dac59670/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/fa02ae6a0b4d0ace080f3eb32febc768dac59670/chrome/browser/ui/views/page_info/page_info_bubble_view.cc
[modify] https://crrev.com/fa02ae6a0b4d0ace080f3eb32febc768dac59670/chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.cc
[modify] https://crrev.com/fa02ae6a0b4d0ace080f3eb32febc768dac59670/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
[modify] https://crrev.com/fa02ae6a0b4d0ace080f3eb32febc768dac59670/ui/views/bubble/bubble_dialog_delegate_view.cc
[modify] https://crrev.com/fa02ae6a0b4d0ace080f3eb32febc768dac59670/ui/views/bubble/bubble_dialog_delegate_view.h

Comment 16 by alancutter@chromium.org, Sep 9

Labels: Merge-Request-70
Requesting to merge fa02ae6a0b4d0ace080f3eb32febc768dac59670 for M70. This fixes permission bubble clipping when PWA windows are maximised on Linux and Windows 7 basic.
Project Member

Comment 17 by sheriffbot@chromium.org, Sep 10

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 18 by bugdroid1@chromium.org, Sep 11 

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

commit ca50bc084195969da2b967a36178ad8108c0cf4a
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue Sep 11 03:35:37 2018

Rename BubbleDialogDelegateView::set_arrow() to SetArrow()

This is a clean up CL for https://chromium-review.googlesource.com/c/chromium/src/+/1201507
which renames BubbleDialogDelegateView::set_arrow() to SetArrow().
This matches our style guide as SetArrow() isn't a simple getter function.

Bug:  877032 
Change-Id: I9d506604d74651deed45a3854356ec7b66e57bb9
Reviewed-on: https://chromium-review.googlesource.com/1203493
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590187}
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/ash/assistant/ui/assistant_container_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/ash/ime/ime_mode_indicator_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/ash/shelf/overflow_bubble_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/ash/system/network/network_state_list_detailed_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/chromeos/lock_screen_apps/toast_dialog_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/cocoa/autofill/save_card_bubble_view_views.mm
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/cocoa/browser_dialogs_views_mac.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/cocoa/tab_dialogs_views_mac.mm
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/ime/ime_warning_bubble_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/intent_picker_bubble_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/page_info/page_info_bubble_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/relaunch_notification/relaunch_recommended_bubble_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/ui/views/bubble/bubble_dialog_delegate_view.cc
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/ui/views/bubble/bubble_dialog_delegate_view.h
[modify] https://crrev.com/ca50bc084195969da2b967a36178ad8108c0cf4a/ui/views/bubble/tooltip_icon.cc
Project Member

Comment 19 by bugdroid1@chromium.org, Sep 12

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1fbc57898e0b59968dc14421e7760534c722c54f

commit 1fbc57898e0b59968dc14421e7760534c722c54f
Author: Alan Cutter <alancutter@chromium.org>
Date: Wed Sep 12 01:15:23 2018

Make page info and permission bubbles anchor from the top right in hosted app windows

This CL fixes a bug where permission prompts and page info dialogs go off screen when
hosted app windows are maximised. This only affects platforms that don't support bubble
anchor adjustment (see calls to set_adjust_if_offscreen(false) in
BubbleDialogDelegateView::CreateBubble()). These platforms are Linux, Mac and Windows 7
with Aero disabled.

This patch fixes the issue by making permission and page info dialogs anchored off
the app menu use TOP_RIGHT instead of TOP_LEFT for their arrow position leading the
bubble to position itself within the main window's bounds by default.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355950&signed_aid=ziHkOFUJALIjs_7uaUaPtQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355951&signed_aid=9B2p0GLHiuHOMC-Ha-rXBQ==&inline=1

Bug:  877032 
Change-Id: I98b1fe1e4fe8018886ae960c8601f5d518c9ea5e
Reviewed-on: https://chromium-review.googlesource.com/1201507
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#589455}(cherry picked from commit fa02ae6a0b4d0ace080f3eb32febc768dac59670)
Reviewed-on: https://chromium-review.googlesource.com/1218203
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#308}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/1fbc57898e0b59968dc14421e7760534c722c54f/chrome/browser/ui/cocoa/browser_dialogs_views_mac.cc
[modify] https://crrev.com/1fbc57898e0b59968dc14421e7760534c722c54f/chrome/browser/ui/views/bubble_anchor_util_views.cc
[modify] https://crrev.com/1fbc57898e0b59968dc14421e7760534c722c54f/chrome/browser/ui/views/bubble_anchor_util_views.h
[modify] https://crrev.com/1fbc57898e0b59968dc14421e7760534c722c54f/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/1fbc57898e0b59968dc14421e7760534c722c54f/chrome/browser/ui/views/page_info/page_info_bubble_view.cc
[modify] https://crrev.com/1fbc57898e0b59968dc14421e7760534c722c54f/chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.cc
[modify] https://crrev.com/1fbc57898e0b59968dc14421e7760534c722c54f/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
[modify] https://crrev.com/1fbc57898e0b59968dc14421e7760534c722c54f/ui/views/bubble/bubble_dialog_delegate_view.cc
[modify] https://crrev.com/1fbc57898e0b59968dc14421e7760534c722c54f/ui/views/bubble/bubble_dialog_delegate_view.h

Comment 20 by kkaluri@chromium.org, Sep 12

Labels: TE-Verified-M71 TE-Verified-71.0.3550.0
Verified this issue on Win 7 & Debian Rodete with chrome #71.0.3550.0 and observed the fix is working as intended.hence adding TE verified labels.

Attaching the screen-cast for reference.
877032.mp4
2.6 MB View Download

Comment 21 by alancutter@chromium.org, Sep 13

Status: Verified (was: Assigned)
The verification video doesn't quite do the right set of steps to repro the bug. It needs to open App Info from the menu.

There's couple more CLs to come relating to this issue but the primary bug where the dialog gets clipped is fixed.
verify.png
32.9 KB View Download
Project Member

Comment 22 by bugdroid1@chromium.org, Sep 13 

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

commit f6f197c3b90dc9ce56f21d084d381f549ec4bf88
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu Sep 13 08:07:07 2018

Fix support for BubbleBorder::NO_SHADOW

Currently when dialog bubbles are created with BubbleBorder::NO_SHADOW
it is no different to being created with BubbleBorder::BIG_SHADOW.
This CL makes NO_SHADOW meaningful by giving it an implementation that
doesn't render a shadow and renders a dark gray border.

This change is in preparation for using NO_SHADOW for dialog bubbles
on platforms that don't support translucent windows.

Existing uses of NO_SHADOW have been changed to BIG_SHADOW to preserve
existing behaviour.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355261&signed_aid=zoH2JbuG4m1h72rb4q92yQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355262&signed_aid=3XnZbPfh4fUBYfOLxYXgHQ==&inline=1

Bug:  877032 
Change-Id: I9d6ca662891bf6c69cba7ad040a7caed7634e81e
Reviewed-on: https://chromium-review.googlesource.com/1195342
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590946}
[modify] https://crrev.com/f6f197c3b90dc9ce56f21d084d381f549ec4bf88/ash/ime/ime_mode_indicator_view.cc
[modify] https://crrev.com/f6f197c3b90dc9ce56f21d084d381f549ec4bf88/chrome/browser/ui/views/feature_promos/feature_promo_bubble_view.cc
[modify] https://crrev.com/f6f197c3b90dc9ce56f21d084d381f549ec4bf88/ui/chromeos/ime/candidate_window_view.cc
[modify] https://crrev.com/f6f197c3b90dc9ce56f21d084d381f549ec4bf88/ui/chromeos/ime/infolist_window.cc
[modify] https://crrev.com/f6f197c3b90dc9ce56f21d084d381f549ec4bf88/ui/views/bubble/bubble_border.cc
[modify] https://crrev.com/f6f197c3b90dc9ce56f21d084d381f549ec4bf88/ui/views/bubble/bubble_border.h
[modify] https://crrev.com/f6f197c3b90dc9ce56f21d084d381f549ec4bf88/ui/views/bubble/bubble_border_unittest.cc
[modify] https://crrev.com/f6f197c3b90dc9ce56f21d084d381f549ec4bf88/ui/views/bubble/bubble_frame_view_unittest.cc
Project Member

Comment 23 by bugdroid1@chromium.org, Sep 13 

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

commit e86f0201184fcd9c399307e3c96ed4a15f9f8a15
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu Sep 13 08:46:24 2018

[Windows 7] Use OS drop shadow for bubble dialogs when Aero mode disabled

If window translucency is not supported by the OS we clip bubble dialogs
to their parent window.
This CL updates our bubble dialogs to use the OS' in build drop shadows
when custom shadow rendering is not supported. This allows for dialog
bubbles to appear outside the main window frame.

This CL only affects Windows 7 non-Aero.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355271&signed_aid=fVEvANP8SRuAP2yenj3FmQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355272&signed_aid=I_IJ0jZ1IwFaT1i_10nBuw==&inline=1

Bug:  877032 
Change-Id: I84628f45d6607254a9b8683dcde40de390db756a
Reviewed-on: https://chromium-review.googlesource.com/1195162
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590953}
[modify] https://crrev.com/e86f0201184fcd9c399307e3c96ed4a15f9f8a15/ash/ime/ime_mode_indicator_view.cc
[modify] https://crrev.com/e86f0201184fcd9c399307e3c96ed4a15f9f8a15/ui/views/bubble/bubble_dialog_delegate_view.cc
[modify] https://crrev.com/e86f0201184fcd9c399307e3c96ed4a15f9f8a15/ui/views/bubble/bubble_dialog_delegate_view.h
[modify] https://crrev.com/e86f0201184fcd9c399307e3c96ed4a15f9f8a15/ui/views/bubble/info_bubble.cc
►

Sign in to add a comment