Issue metadata
Sign in to add a comment
|
Regression:Unnecessary space is seen between text and 'Discard' button on download shelf.
Reported by
dmascare...@etouch.net,
Nov 24 2016
|
||||||||||||||||||||||
Issue descriptionChrome Version:57.0.2929.3 (Official Build)3619fbd7f57a1d171fac4d4adedde499f17eec95-refs/branch-heads/2929@{#3} 32/64-bit. OS:Windows (7,8,10) What steps will reproduce the problem? 1. Launch chrome and navigate to http://parkerly.com/sb-tests/downloads/downloads.html 2. Click on 'badfile.exe -> somefile.exe' link and observe at download shelf. Actual:Unnecessary space is seen between text and 'Discard' button on download shelf. Expected:Space between text and 'Discard' button should be proper on download shelf. This is regression issue, broken in 'M 56' and below manual bisect info: Good build:56.0.2900.0 Bad build:56.0.2902.0 Note: Issue is not seen on Mac and Linux OS.
,
Nov 24 2016
My CL only changed warning strings, should not impact download waning layout. Could you assign it to an UI expert to take a look?
,
Dec 5 2016
Just to update, still able to reproduce the issue on windows 7 using chrome version 57.0.2939.0
,
Dec 5 2016
I took another look, it seems my CL triggered this regression, but not caused it. The true reason is SizeLabelToMinWidth() function in download_item_view.cc (which hasn't been touched since 2013) seems not working probably with the new string (on Windows). I'll try to fix this function in this week.
,
Dec 5 2016
,
Dec 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/047611df69b7f337f96d4ce684d21cb69baeb0b8 commit 047611df69b7f337f96d4ce684d21cb69baeb0b8 Author: jialiul <jialiul@chromium.org> Date: Fri Dec 09 16:51:55 2016 Fix SizeLabelToMinWidth() function such that no unnecessary space between warning label and Discard button on download shelf. Note, this bug has been there for more than 3 years. The string change in crrev.com/2443343002 exposed this issue. The reason this bug was only reproducible on Win is because Win's font is narrower than other OSs' (a.k.a label with the same text on Win yields a smaller width) BUG= 668472 Review-Url: https://codereview.chromium.org/2556573002 Cr-Commit-Position: refs/heads/master@{#437563} [modify] https://crrev.com/047611df69b7f337f96d4ce684d21cb69baeb0b8/chrome/browser/ui/views/download/download_item_view.cc [modify] https://crrev.com/047611df69b7f337f96d4ce684d21cb69baeb0b8/chrome/browser/ui/views/download/download_item_view.h [add] https://crrev.com/047611df69b7f337f96d4ce684d21cb69baeb0b8/chrome/browser/ui/views/download/download_item_view_unittest.cc [modify] https://crrev.com/047611df69b7f337f96d4ce684d21cb69baeb0b8/chrome/test/BUILD.gn
,
Dec 9 2016
,
Dec 10 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fdc8672287946320d9547ddbc9bc29d721e30459 commit fdc8672287946320d9547ddbc9bc29d721e30459 Author: Jialiu Lin <jialiul@chromium.org> Date: Sat Dec 10 18:58:25 2016 Fix SizeLabelToMinWidth() function such that no unnecessary space between warning label and Discard button on download shelf. Note, this bug has been there for more than 3 years. The string change in crrev.com/2443343002 exposed this issue. The reason this bug was only reproducible on Win is because Win's font is narrower than other OSs' (a.k.a label with the same text on Win yields a smaller width) BUG= 668472 Review-Url: https://codereview.chromium.org/2556573002 Cr-Commit-Position: refs/heads/master@{#437563} (cherry picked from commit 047611df69b7f337f96d4ce684d21cb69baeb0b8) Review-Url: https://codereview.chromium.org/2560403003 . Cr-Commit-Position: refs/branch-heads/2924@{#448} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/fdc8672287946320d9547ddbc9bc29d721e30459/chrome/browser/ui/views/download/download_item_view.cc [modify] https://crrev.com/fdc8672287946320d9547ddbc9bc29d721e30459/chrome/browser/ui/views/download/download_item_view.h [add] https://crrev.com/fdc8672287946320d9547ddbc9bc29d721e30459/chrome/browser/ui/views/download/download_item_view_unittest.cc [modify] https://crrev.com/fdc8672287946320d9547ddbc9bc29d721e30459/chrome/test/BUILD.gn
,
Dec 14 2016
Verified the issue on windows 10 using chrome beta version #56.0.2924.28 as per comment #0 Observed that space between text and 'Discard' button was proper on download shelf. Hence, the fix is working as expected. Attaching screenshot for reference. Hence, adding the verified labels. Thanks...!! |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by kkaluri@chromium.org
, Nov 24 2016Labels: hasbisect-per-revision
Owner: jialiul@chromium.org
Status: Assigned (was: Unconfirmed)