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

Issue 667698 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 695577



Sign in to add a comment

Regression : Unable to resize the browser to its minimum level after adding an ‘Darkness’ extension.

Reported by yfulgaon...@etouch.net, Nov 22 2016

Issue description

Chrome Version : 56.0.2924.3 (Official Build) 66f73b3ae5a592d70e00602e87a783535ffe967e-refs/branch-heads/2924@{#54} 64-bit
OS : Mac(10.11.6, 10.12.1, 10.12)

Test URL : https://chrome.google.com/webstore/detail/darkness-beautiful-dark-t/imilbobhamcfahccagbncamhpnbkaenm/related?utm_source=chrome-ntp-icon

What steps will reproduce the problem?
1. Launch chrome, navigate to above URL and add an ‘Darkness’ extension (‘Welcome to darkness’ Tab gets opened).
2. Now try to resize browser by dragging inwards from RHS and observe. (Please review an attached screen cast for the reference)

Actual : Unable to resize the browser to its minimum level after adding an ‘Darkness’ extension.
Expected : User should be able to resize the browser to its minimum level.

This is a regression issue broken in ‘M-56’, below is the Manual Regression range and will soon update other info.
Good Build : 56.0.2886.0
Bad Build : 56.0.2888.0

Note : This is Mac specific issue and the same is working fine on Windows & Linux OS.
 
Actual_browser_resize.mov
7.7 MB Download
Expected_browser_resize.mov
8.7 MB Download
Labels: hasbisect-per-revision
Owner: eugene...@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 56.0.2886.0 (Revision: 424099).
Bad build: 56.0.2888.0 (Revision: 424625).

You are probably looking for a change made after 424608 (known good), but no later than 424609 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/751e1f52e94d3d611e27571de632b6a8faa7fc34..42c6683729d8e049fdcc89e552bdcff5fb337227

@eugenebut -- Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Thank You.

Cc: eugene...@chromium.org
Owner: tapted@chromium.org
Trent, did we disable my "fix"?

Comment 3 by tapted@chromium.org, Nov 23 2016

Blocking: 605219
Yup, in r433658 but it needs merging (r433658 is in 2927 which hasn't hit Canary yet, so not eligible).

And if this was reported in 2924 then it probably wasn't fixed by r432024, which is in 2920. So I'll set this as a blocker for re-enabling.

Comment 4 by tapted@chromium.org, Nov 23 2016

Cc: tapted@chromium.org
Labels: -M-56 M-57
Owner: ----
Status: Available (was: Assigned)
I'll do the merge in  Issue 666415  which is releaseblock-beta

Moving this to available since it's not in my queue.

Comment 5 Deleted

Comment 6 by hdodda@chromium.org, Nov 28 2016

Labels: -TE-Verified-57.0.2934.0 TE-Verified-57.0.2935.0
Tested the fix on Mac 10.11.6 using chrome version 57.0.2935.0 with the steps mentioned in comment #0.
Able to resize the window from RHS after adding the extension.
Please find the attched screen cast for the same.

Adding TE-Verified labels.
Thanks!
667698.mp4
1.4 MB View Download
Shouldn't this be marked as fixed?

Comment 8 by tapted@chromium.org, Nov 28 2016

The current fix for this issue is a revert (r433658) of something we want to reland ( Issue 605219 ). So this issue still needs a proper fix.
Labels: -TE-Verified-M57 -TE-Verified-57.0.2935.0
As per Comment#8, removing TE Verified labels.

tapted@ Please let us know once proper fix get landed.

Thank you.

Owner: tapted@chromium.org
@tapted -- Any latest updates regarding the Fix, please let us know so that the issue would be checked on latest builds and changed to Fixed state.
Thank You.
Cc: sdy@chromium.org
Labels: -Pri-1 -Type-Bug-Regression -M-57 Pri-3 Type-Bug
Owner: ----
I'm not currently working on a fix. A fix isn't what's needed: we just need to ensure this bug doesn't happen again when resolving the blocked,  issue 605219 .

Comment 13 by sdy@chromium.org, Mar 7 2017

Blocking: -605219 695577
Owner: sdy@chromium.org
Status: Started (was: Available)
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 15 2017

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

commit d06b1c5641ca1022fd4165ea46d8295fb5a215b4
Author: sdy <sdy@chromium.org>
Date: Wed Mar 15 11:32:41 2017

[Mac] Lay out the browser window when adding the download shelf.

## Summary
This fixes a regression when a browser window uses Auto Layout: if the download
shelf is added to the window without ever being shown (e.g. when installing an
extension), then the window can't be made as narrow as usual.

## Explanation
Before this CL, BrowserWindowController doesn't lay out the window after adding
the download shelf. When the download shelf is shown, -layoutSubviews is called
via -resizeView:newHeight:, but in some cases, like a quick extension install,
it's added and left hidden. In that case, the shelf will be positioned like
this (but you can't see it, since it's hidden and has zero height):

    /---------------------------------------\
    |---------------------------------------|
    |                                       |
    |                                       |
    |                                       |
    |                                       |
    |                                       |
    |                                       |
    |                                       |
    |---------------------------            |
    |                         x|            |
    \---------------------------------------/

The shelf's autoresizing mask has a flexible width but fixed margins. If the
window is wider than the shelf when it's added, then it will have a right
margin. As the user makes the window narrower, the download shelf is made
narrower and eventually reaches a minimum size.

If the user makes the window even smaller, one of two things will happen
depending on whether Auto Layout is handling the window:

- No Auto Layout: the layout breaks (but the user never sees it happen, because
  -layoutSubviews fixes it up if the download shelf is shown).

- Auto Layout: The layout engine treats the fixed margin as a constraint and
  prevents the window from becoming any narrower.

The next time -[BrowserWindowController layoutSubviews] runs, the download
shelf is made the same width as the window, and the problem goes away because
it no longer has a margin.

## Future work
Only add the download shelf to the view hierarchy when it's visible.

BUG= 667698 ,  695577 

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

[modify] https://crrev.com/d06b1c5641ca1022fd4165ea46d8295fb5a215b4/chrome/browser/ui/cocoa/browser_window_controller.h
[modify] https://crrev.com/d06b1c5641ca1022fd4165ea46d8295fb5a215b4/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/d06b1c5641ca1022fd4165ea46d8295fb5a215b4/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm

Comment 15 by sdy@chromium.org, Mar 16 2017

Status: Fixed (was: Started)

Sign in to add a comment