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

Issue 874828 link

Starred by 1 user
Status: Fixed
Owner:
█ sweilun@chromium.org
Last visit 18 days ago
Closed: Aug 29
Cc:
pnangunoori@chromium.org
nyerramilli@chromium.org
rbasuvula@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug-Regression



Sign in to add a comment

Regression: 'Customize this page' overlay closes after clicking bottom part of the overlay

Reported by khushal....@etouch.net, Aug 16 
Chrome Version: 69.0.3497.42 (Official Build) Revision 9c4613c66dfeb2b76ef6dd4b15884c15db3b4969-refs/branch-heads/3497@{#655} (32/64-bit)
OS: Mac (10.12.6, 10.13.1, 10.13.6, 10.14), Win (7, 8, 8.1, 10) & Linux (14.04 LTS)

Pre-condition: Enable the flag 'Enable using the Google local NTP' and 'New Tab Page Background Selection' from chrome://flags/

What steps will reproduce the problem?
(1) Launch chrome, open NTP and click on gear icon ('Customize this page' overlay will appear).
(2) Now click on bottom part (below last option) of the overlay and Observe.

Actual Result: 'Customize this page' overlay closes after clicking bottom part of the overlay.
Expected Result: 'Customize this page' overlay should not close after clicking bottom part of the overlay.

This is a Regression issue seen from 'M-69' and providing the bisect info below:
Good Build: 69.0.3473.0 (Revision: 570289)
Bad Build:  69.0.3474.0 (Revision: 570647)

You are probably looking for a change made after 570432 (known good), but no later than 570433 (first known bad).

CHANGE-LOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/a599f10a42b637d6dbf03e805894e59fc4d5f79f..c13751e339395d0c9d454786c419206b926e7b5a

Suspect: https://chromium.googlesource.com/chromium/src/+/c13751e339395d0c9d454786c419206b926e7b5a

@kmilka: 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 also seen on M-70 Dev (build #70.0.3521.2) & M-70 Canary (build #70.0.3524.0).

Kindly refer the attached screen-cast.

Thank You..!!
Actual Video.mov
6.6 MB View Download
Expected Video.mov
5.5 MB View Download

Comment 1 by ramyan@chromium.org, Aug 16

Labels: -Pri-1 -Target-69 Pri-3
Owner: sweilun@chromium.org
I think it's probably applying the resetBackground onclick listener. From a usability perspective, clicking on a disabled menu item shouldn't result in an action.

Comment 2 by sweilun@chromium.org, Aug 16 

Now, when clicking the dialog or even dragging the mouse will not close the dialog. However, in the last second in the screen cast, you will see when clicking the bottom of the dialog will still close the dialog. One way to fix it is remove the dialog's padding. But I am not sure if it is appropriate. wdyt
 
Screencast:
https://screencast.googleplex.com/cast/NDY5Mjc5MDU3OTU2MDQ0OHxhOWVlODk3OC0zMg

Comment 3 by sweilun@chromium.org, Aug 20

Status: Started (was: Assigned)

Comment 4 by twelling...@chromium.org, Aug 21

Labels: zine-triaged

Comment 5 by ramyan@chromium.org, Aug 22 

I'm not sure I understand how the padding affects this. I think checking whether the menu item is bg-option-disabled before applying editDialog.close here would help: https://cs.chromium.org/chromium/src/chrome/browser/resources/local_ntp/custom_backgrounds.js?g=0&l=864 ?

Comment 6 by ramyan@chromium.org, Aug 22 

Two issues here: 
1. disabled menu item can be clicked & closes the dialog. (fixable)
2. padding around the border of the menu item is clickable & closes the menu (Weilun is reviewing).
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 29 

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

commit 153fc387e12b2516a395f49ef06a7417fb06bcc7
Author: Weilun Shi <sweilun@chromium.org>
Date: Wed Aug 29 22:22:01 2018

[NTP] Clicking on the customize this page dialog will not close the dialog

Adding an additional div container on top of the "customize this page"
dialog can make sure the dialog will close only when we click outside
the dialog. Also, clicking on the disabled option will no longer close
the dialog.

Bug:  874828 
Change-Id: Ibe16d3e898feefa716cf09e49a52c8a788de254c
Reviewed-on: https://chromium-review.googlesource.com/1195716
Reviewed-by: Kristi Park <kristipark@chromium.org>
Commit-Queue: Weilun Shi <sweilun@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587331}
[modify] https://crrev.com/153fc387e12b2516a395f49ef06a7417fb06bcc7/chrome/browser/resources/local_ntp/custom_backgrounds.css
[modify] https://crrev.com/153fc387e12b2516a395f49ef06a7417fb06bcc7/chrome/browser/resources/local_ntp/custom_backgrounds.js
[modify] https://crrev.com/153fc387e12b2516a395f49ef06a7417fb06bcc7/chrome/browser/resources/local_ntp/local_ntp.html

Comment 8 by sweilun@chromium.org, Aug 29

Status: Fixed (was: Started)

Comment 9 by khushal....@etouch.net, Aug 31

Labels: TE-Verified-M70 TE-Verified-70.0.3538.0
Update:

Rechecked the above issue on Mac (10.12.6, 10.13.1, 10.13.6, 10.14), Win (7, 8, 8.1, 10) & Linux (14.04 LTS) using latest canary version #70.0.3538.0 and the issue is found FIXED.
Hence, adding the respective labels.

Please refer the attached screen-cast.

Thank You..!!
874828_FIXED.mov
1.6 MB View Download

Comment 10 by ramyan@chromium.org, Sep 10

Labels: AddToRemoteNTP

Comment 11 by kristip...@chromium.org, Nov 15

Labels: -AddToRemoteNTP SupportedInRemoteNTP
►

Sign in to add a comment