New issue
Advanced search Search tips

Issue 726184 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug-Regression
Team-Security-UX


Show other hotlists

Hotlists containing this issue:
EnamelAndFriendsFixIt


Sign in to add a comment

Material permission prompts are very wide

Project Member Reported by lgar...@chromium.org, May 25 2017

Issue description

Chrome 60.0.3107.4
OSX 10.12.4

What steps will reproduce the problem?
(1) Enable --secondary-ui-md
(1) Visit permission.site and trigger a prompt.

What is the expected result?
The prompt is similar to cocoa.png

What happens instead?
The prompt is very wide (observed.png).

A biset shows that we changed from expected.png to observed.png in https://codereview.chromium.org/2821413002

elllyjones@, could you triage as either a regression or WontFix if this is intended?
(I'd be interested in knowing why, if it's intended.)
 
expected.png
276 KB View Download
observed.png
44.7 KB View Download
cocoa.png
54.3 KB View Download
That is because of the Harmony dialog width snapping. I am not sure if it is intentional per se, but it is the width snapping performing as designed. The dialog's natural bounds on my Linux system are 352x129, which we snap up to 448x129. I will ask the designers about this dialog.
The root cause why the dialog is 352x129 is this: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc?l=309

it sets the PermissionsBubbleDialogDelegateView's width to 320, then we add the usual bubble margins, get 352, and round up to 448, so that's bad. If I remove that, though, the bubble collapses down to a tiny size in non-Harmony mode, because the title is wide and the permission strings are narrow. Awkward.

Comment 3 by tapted@chromium.org, May 26 2017

A bubble won't know how wide it is until it's in a Widget, and GetPreferredSize() needs to work before there is a Widget, so the fragment,

gfx::Size PermissionsBubbleDialogDelegateView::GetPreferredSize() const {
  // TODO(estade): bubbles should default to this width.
  const int kWidth = 320 - GetInsets().width();
  return gfx::Size(kWidth, GetHeightForWidth(kWidth));
}

won't do the right thing before or after Harmony.

We can, perhaps, put a helper on BubbleDialogDelegateView with a hard-coded constant that provides the additional width that the default NonClientFrameView will add to the edges of bubbles. That should help bubbles transition over to harmony before they all get a useful default minimum width.

But also it seems reasonable just to impose the min width of 320 when ShouldSnapFrameWidth() returns true pre-harmony as well. That will let us just delete some of these GetPreferredSize() overrides.
Project Member

Comment 4 by bugdroid1@chromium.org, May 31 2017

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

commit 436d5e22db0b978fc69612f62f8a9432622d8fe1
Author: ellyjones <ellyjones@chromium.org>
Date: Wed May 31 13:59:07 2017

views: impose minimum width on non-Harmony snappables

Some dialogs, like the permission bubble, have very narrow
preferred contents view widths, but titles that require wider
dialogs. Instead of these dialogs individually imposing
minimum widths, require all snappable non-Harmony dialogs to
have a minimum width.

BUG= 726184 

Review-Url: https://codereview.chromium.org/2907873002
Cr-Commit-Position: refs/heads/master@{#475900}

[modify] https://crrev.com/436d5e22db0b978fc69612f62f8a9432622d8fe1/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
[modify] https://crrev.com/436d5e22db0b978fc69612f62f8a9432622d8fe1/ui/views/layout/layout_provider.cc

Comment 5 by est...@chromium.org, Nov 10 2017

Labels: Hotlist-EnamelAndFriendsFixIt
Status: Fixed (was: Assigned)
This got fixed a while ago by 12d3545cdca172bf6e8f1fb7e831a2d12c916f26.

Sign in to add a comment