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

Issue 637882 link

Starred by 6 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Notification button does not show properly when shelf orientation changes

Project Member Reported by yiyix@chromium.org, Aug 15 2016

Issue description

Version: Chrome @411418
OS: Chrome OS

What steps will reproduce the problem?
(1) build chrome
(2) change shelf orientation from bottom to left 

What is the expected output?
The background of notification button does not change size as shelf alignment changes.

What do you see instead?
The height of the background of the notification button is decrease.
When the shelf is set to bottom, then the background of the notification button is decreased significantly. 

Please use labels and text to provide additional information.


 

Comment 1 by yiyix@chromium.org, Aug 15 2016

Screenshot from 2016-08-15 14:03:11.png
4.9 KB View Download
Screenshot from 2016-08-15 14:02:50.png
16.2 KB View Download
Cc: tdander...@chromium.org
Components: UI>Shell>Shelf
Labels: -Type-Bug -Pri-3 M-54 OS-Chrome Pri-1 Type-Bug-Regression
Status: Assigned (was: Untriaged)

Comment 3 by yiyix@chromium.org, Aug 15 2016

Description: Show this description

Comment 4 by yiyix@chromium.org, Aug 15 2016

Owner: yiyix@chromium.org
https://bugs.chromium.org/u/yoshiki@chromium.org/
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 15 2016

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

commit e311188e565ea85ccc4ad8f0aeee03eab488bb23
Author: yiyix <yiyix@chromium.org>
Date: Mon Aug 15 21:53:27 2016

Fix notification button background size

In CL2243563002, a new variable |margin_| was introduced to TrayBackgroundView
to adjust the size of notification button background; however, this adjustment
is later cancelled by re-setting border to NULL. As a result, the background
is incorrect as shelf alignment changes.

BUG= 637882 

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

[modify] https://crrev.com/e311188e565ea85ccc4ad8f0aeee03eab488bb23/ash/common/system/web_notification/web_notification_tray.cc

Labels: -M-54 ReleaseBlock-Stable M-53 Merge-Request-53
Thank you for fixing that. The original issue (Issue 633399) is for M53, so let me change the target.
yoshiki@, are you going to handle the merge back into 53? If so please mark yourself as the owner of this bug.

Comment 8 by dimu@chromium.org, Aug 16 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Cc: yiyix@chromium.org
Owner: yoshiki@chromium.org
Status: Started (was: Assigned)
Let me merge.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 18 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c1257266665f6b80473a17347edfda7fd7f57f9

commit 9c1257266665f6b80473a17347edfda7fd7f57f9
Author: yoshiki iguchi <yoshiki@chromium.org>
Date: Thu Aug 18 01:12:10 2016

Fix notification button background size

In CL2243563002, a new variable |margin_| was introduced to TrayBackgroundView
to adjust the size of notification button background; however, this adjustment
is later cancelled by re-setting border to NULL. As a result, the background
is incorrect as shelf alignment changes.

BUG= 637882 

Review-Url: https://codereview.chromium.org/2247893002
Cr-Commit-Position: refs/heads/master@{#412060}
(cherry picked from commit e311188e565ea85ccc4ad8f0aeee03eab488bb23)

Review URL: https://codereview.chromium.org/2256053002 .

Cr-Commit-Position: refs/branch-heads/2785@{#650}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/9c1257266665f6b80473a17347edfda7fd7f57f9/ash/common/system/web_notification/web_notification_tray.cc

Status: Fixed (was: Started)
Merged. Thank you for fixing this!
Status: Verified (was: Fixed)
Verified on ChromeOS:8530.63.0/chrome:53.0.2785.72
Cc: abodenha@chromium.org xiy...@chromium.org rookrishna@chromium.org abod...@chromium.org dhadd...@chromium.org yoshiki@chromium.org
 Issue 639020  has been merged into this issue.

Sign in to add a comment