Issue metadata
Sign in to add a comment
|
Regression: Toolbar height flickers/enlarges during Canary is starting |
||||||||||||||||||||||
Issue descriptionChrome Version: Canary 73.0.3644.0 OS: macOS 10.14.2 What steps will reproduce the problem? (1) Quit Canary (2) Start Canary (3) Take a look at the Toolbar during Canary starts What is the expected result? Toolbar should open in its fixed height. What happens instead? Toolbar height flickers/enlarges. A screencast is attached. dfried@: Probably caused by your latest Toolbar Layout changes?! Can you please take a look? If you want, I can bisect it to be sure that it is caused by this change. A screencast is attached. Thanks :)
,
Dec 18
The problem here is that the whole Browser area Tabstrip/Toolbar/Bookmarks Bar has been increased by some px now :( Attached a screenshot. Below Stable, Top Canary.
,
Dec 18
Okay, the main problem here is the Toolbar. It is at all 4px higher than before now. In detail, it is increased by 2px above and 2px below the Omnibox area now.
,
Dec 18
This does appear to be the result of the toolbar now respecting the preferred sizes of its child views. I will determine which child view is at fault and make sure it's reporting correctly.
,
Dec 18
Okay, thanks in advance :)
,
Dec 18
Height is too high because we are applying a uniform inset around each icon in the browser actions (extensions) bar, which includes top and bottom, which makes that bar taller than the other views in the toolbar, which causes the toolbar to be too large when the browser actions view is visible. We need to do a better job of reporting the size of the BrowserActionsContainer. Handing off to pbos@
,
Dec 26
Friendly ping to get an update as it is marked as RBS. Thanks..!
,
Jan 2
Gemtle ping to get an update as it is marked as RBS. Thanks..!
,
Jan 2
,
Jan 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/82848abbf09b0cf470a2f06a617fe681d87f7f8a commit 82848abbf09b0cf470a2f06a617fe681d87f7f8a Author: Peter Boström <pbos@chromium.org> Date: Fri Jan 04 18:00:33 2019 Decrease extension button size + add spacing This makes ToolbarActionView the same size as other toolbar buttons instead of increasing its internal size with 4dp to accomodate for padding between items. This reduces the BrowserActionsContainer height by 4dp (matching its current visual height) so it should no longer be able to push the toolbar height. It also fixes some polish where the spacing left and right of the browser actions were inadvertedly 10dp instead of 8dp. Bug: chromium:831393 , chromium:916241 , chromium:916462 Change-Id: I63e5413ce187f173a5e35b40a3ec8f02a68fa09c Reviewed-on: https://chromium-review.googlesource.com/c/1394870 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#619999} [modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/layout_constants.cc [modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc [modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc [modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/views/toolbar/browser_actions_container.cc [modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/views/toolbar/browser_actions_container.h
,
Jan 4
This should be fixed now. mehmet@ if could you validate the next Canary, please let us know. Thanks!
,
Jan 4
,
Jan 4
Awesome! Verified the fix in Snapshot #620014. Thank you :)
,
Jan 8
Issue 917187 has been merged into this issue. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by meh...@chromium.org
, Dec 18