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

Issue 735889 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: "X" icon is seen by default on extension crash bubble

Project Member Reported by sc00335...@techmahindra.com, Jun 22 2017

Issue description

Chrome Version: 61.0.3138.0 dev
OS: Ubuntu 14.04,Windows

What steps will reproduce the problem?
(1) Launch chrome and add any extension that can be seen on browser containment area [Ex: https://chrome.google.com/webstore/detail/grammarly-for-chrome/kbfnbcaeplbcioakkpcpgfkobkghlhen?hl=en-GB ]
(2) Now open task manager and end that extension process and observe crash bubble of extension

Expected: "x" icon should be seen only when hovered on it.
Actual: Instead "x" icon is seen by default and is seen overlapped with content in some extension bubbles.

Manual Bisect Info:
===================
Good Build: 61.0.3129.0 dev
Bad Build: 61.0.3130.0 dev
 
Actual_icon.png
230 KB View Download
Expected_x icon.ogv
834 KB View Download
Actual_x icon.ogv
875 KB View Download
Labels: OS-Mac
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on Mac 10.12.5 and Win-10 using latest canary #61.0.3138.0.

Thanks...!!
Cc: jmukthavaram@chromium.org
Labels: -Needs-Bisect hasbisect-per-revision
Able to reproduce the issue on Windows 7, mac & Ubuntu 14.04 using latest canary #61.0.3138.0.

Manual Bisect Info:
===================
Good Build: 61.0.3129.0-Revision-478840
Bad Build: 61.0.3130.0 -Revision-479232

Per revision bisect info:
------------------------
You are probably looking for a change made after 478882 (known good), but no later than 478902 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspectas some perf builds might get missing due to failure.
https://chromium.googlesource.com/chromium/src/+log/a113781033a747eaf60e8ad7799e9b95a58ceed4..b13657fa54d21665d049cf2089f3b861de07d291

Unable to find the exact suspect from the above CL.

Could anyone from the dev team please look into this issue.

Thanks in advance..!!

Comment 3 by ajha@chromium.org, Jun 22 2017

Owner: yhanada@chromium.org
Status: Assigned (was: Untriaged)
yhanada@: Could this be related to https://chromium-review.googlesource.com/c/530726/.
Cc: yoshiki@chromium.org
Components: UI>Shell>Notifications
Thank you for reporting.

I changed the default visibility of the close button on pop-ups to address  crbug.com/726812 .
Is there any other problem to move the button back to the lower-right corner as before M-59?
The "x" button is seen overlapped with text in screenshot. You can either fix that or can revert back to M59 design.
Owner: sgabr...@chromium.org
sgabriel@: What would you suggest to fix this issue? Is it ok to reduce the width of the title not to be covered by the button?
this solution sounds good. try to put something like 8px padding between title label and X button container.
Owner: yhanada@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 16 2017

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

commit 97156ad7e3aaf12a766c5e69d457ad15f492690b
Author: yhanada <yhanada@chromium.org>
Date: Wed Aug 16 05:58:13 2017

Reduce width of top most view in a notification not to be covered by the control buttons.

Bug:  735889 
Test: Manual. Show a notification with log title and check that the
      title text is not covered by the control buttons.

Change-Id: Ic9598e1b4873d1f130eda0eb993bfe2eed1a82d4
Reviewed-on: https://chromium-review.googlesource.com/607746
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494704}
[modify] https://crrev.com/97156ad7e3aaf12a766c5e69d457ad15f492690b/ui/message_center/views/notification_view.cc
[modify] https://crrev.com/97156ad7e3aaf12a766c5e69d457ad15f492690b/ui/message_center/views/notification_view.h

Labels: Merge-Request-61
I request merge the change in #10 to M-61.
Project Member

Comment 12 by sheriffbot@chromium.org, Aug 17 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Before we approve merge to M61, please answer followings:
* This seems to be M61 regression, how critical it is?
* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61?
* Any other important details to justify the merge.

Please note We're only few weeks away from M61 Stable promotion, so merge bar is very high.

Comment 14 by abom...@etouch.net, Aug 17 2017

Note:Above issue is still reproducible on Latest Canary version:62.0.3188.0 (Official Build) using Linux,Windows and Mac OS

Thanks You.
Actual_X.mp4
708 KB View Download
Thank you for reviewing!
A close button a settings button can be overlapping on text of a notification without the fix. This makes it harder for users to read content of a notification.
The change is in 62.0.3188.0. We should watch for one or two days. I'm sorry for requesting merge too early.

The change fixed only the overlapping issue.
Visibility of a close button and a settings button is working as intended. Please see  crbug.com/726812 .

Thanks!
Thank you  yhanada@.Per comment #14, this issue doesn't seem to be fixed on Canary version 62.0.3188.0. Could you please double check?

 

As discussed in  crbug.com/726812 , always showing up close button and settings button is the intended behavior.

The CL fixes the problem that these buttons overlap text of a notification.

I found that there is another minor issue in layouting views in notificaiton, so I'll try to fix the all issues completely in M-62.
I'm withdrawing the merge request. Sorry for the confusion.
Labels: -Merge-Review-61 M-62 Merge-Rejected-61
Rejecting merge to M61 based on comment #17.
Status: Fixed (was: Started)
The issue I mentioned in #17 is tracked in  crbug.com/785589 .

Sign in to add a comment