New issue
Advanced search Search tips

Issue 851843 link

Starred by 1 user
Status: Fixed
Owner:
ftirelo@chromium.org
Closed: Jun 2018
Cc:
nyerramilli@chromium.org
rbasuvula@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression


Show other hotlists

Hotlists containing this issue:
Hotlist-4


Sign in to add a comment

Regression: Unwanted extra space is seen on LHS on wrench menu.

Reported by khushal....@etouch.net, Jun 12 2018 
Chrome Version : 69.0.3455.0 (Official Build) Revision f981d5ba6963015157ac5faa8090cd22bec65a3c-refs/branch-heads/3455@{#1} (32/64-bit)
OS: Win (7, 8, 8.1, 10) & Linux (14.04 LTS)

Steps to reproduce:
1. Launch chrome and navigate to NTP.
2. Now click on wrench menu and observe the space on LHS of menu list.

Actual Result: Unwanted extra space is seen on LHS on wrench menu.
Expected Result: LHS of wrench menu should have proper space.

This is Regression issue broken in 'M-69’ and providing the bisect info below:

Good Build: 69.0.3452.0 (Revision: 565144)
Bad Build:  69.0.3453.0 (Revision: 565531)

Narrow bisect URL:

https://chromium.googlesource.com/chromium/src/+log/76257d1ab79276b2d53ee976b2c3e3b9f335cde7..4ea283bffe496b27dd41051c0348ff2600e0bab0?pretty=fuller&n=10000

Suspect: https://chromium.googlesource.com/chromium/src/+/4ea283bffe496b27dd41051c0348ff2600e0bab0

@ftirelo: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Note: Issue is not seen on Mac OS. 

Kindly refer attached screen-cast.

Thank You..!!
Actual Video.mp4
1.2 MB View Download
Expected Video.mp4
1.1 MB View Download
Screenshot.png
35.0 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 20 2018 

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

commit 8bcae93189b36a82d8f20749e3935ce025cb48ae
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Wed Jun 20 22:32:03 2018

Stop using layout provider for menu horizontal spaces

Menu horizontal spaces were consolidated into a single
item_horizontal_padding by crrev.com/c/1089431, which also made the
value come from the layout provider.

However, when Harmony is enabled, the horizontal padding becomes too
large (8 instead of 16). As a side effect, menus became too spacey on
Harmony (see details on the linked bug).

One alternative was to use DISTANCE_RELATED_CONTROL_HORIZONTAL_SMALL and
define the corresponding padding as 8 for Harmony, but that would
introduce a dependency from ui/views/controls to chrome/browser/ui/views,
which I don't think is desirable. In addition, while Harmony is rolling
out, making sure we don't break paddings on other browser components can
be challenging, so I added a TODO to make a general consolidation once
Harmony becomes the default.

Bug:  851843 
Change-Id: Ifbecf12f434e20af8f93b590feed7eb4b2bc33de
Reviewed-on: https://chromium-review.googlesource.com/1108451
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569050}
[modify] https://crrev.com/8bcae93189b36a82d8f20749e3935ce025cb48ae/chrome/browser/ui/app_list/app_context_menu_unittest.cc
[modify] https://crrev.com/8bcae93189b36a82d8f20749e3935ce025cb48ae/ui/views/controls/menu/menu_config.cc
[modify] https://crrev.com/8bcae93189b36a82d8f20749e3935ce025cb48ae/ui/views/controls/menu/menu_item_view_unittest.cc
[modify] https://crrev.com/8bcae93189b36a82d8f20749e3935ce025cb48ae/ui/views/layout/layout_provider.cc
[modify] https://crrev.com/8bcae93189b36a82d8f20749e3935ce025cb48ae/ui/views/layout/layout_provider.h

Comment 2 by ftirelo@chromium.org, Jun 20 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment