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

Issue 881781 link

Starred by 1 user
Status: Fixed
Owner:
kristip...@chromium.org
Closed: Sep 13
Cc:
yyushkina@chromium.org
nyerramilli@chromium.org
rbasuvula@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression


Participants' hotlists:
NTP-Birthday-Polish


Sign in to add a comment

Regression : Unable to dismiss "Shortcut removed" message by clicking on 'X' icon on Yahoo NTP.

Reported by avsha...@etouch.net, Sep 7 
Chrome Version : 71.0.3545.0 (Official Build) 38ca181b5b0849226f5f15de263301b40cd36b5d-refs/branch-heads/3545@{#1} 32/64 bit
OS : Windows (7, 8, 8.1, 10), Linux(14.04 LTS), Mac(10.12.6, 10.13.1, 10.14, 10.13.6)

Precondition : Enable '#ntp-icons' and '#use-google-local-ntp' flags from chrome://flags.

What steps will reproduce the problem?
1. Launch chrome, enable above flags and relaunch chrome.
2. Go to chrome://settings/searchEngines and change default 'search engine' to 'Yahoo'.
3. Open NTP and remove any existing thumbnail by using 'x' icon. ("Shortcut removed" message appears at the bottom)
4. Now try to dismiss "Shortcut removed" message by clicking on 'X' icon and observe.

Actual Result : Unable to dismiss "Shortcut removed" message by clicking on 'X' icon on Yahoo NTP.

Expected Result : "Shortcut removed" message should get dismissed when user click on 'X' icon on Yahoo NTP.

This is a regression issue broken in ‘M-68’ and below is the 'per-revision' bisect information:
Good Build : 68.0.3439.0 (Revision : 561389)
Bad Build : 68.0.3440.0 (Revision : 561733)

Change Log URL :
https://chromium.googlesource.com/chromium/src/+log/3d8da92027fb6e434d47975194d69004122ce5b7..91da70f4745997eff76e97444c553114e1006b6d

Suspecting : https://chromium.googlesource.com/chromium/src/+/91da70f4745997eff76e97444c553114e1006b6d

kristipark@ : 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 : 
This issue is also observed in latest Dev build #70.0.3538.11 and Stable #69.0.3497.81

Thank you..!
Expected_Result.mp4
529 KB View Download
Actual_Result.mp4
767 KB View Download

Comment 1 by kristip...@chromium.org, Sep 10

Cc: yyushkina@chromium.org ramyan@chromium.org
Undo and Restore Default functionality is also broken for third party NTPs.

Comment 2 by kristip...@chromium.org, Sep 10 

Addendum, Undo and Restore Default are only broken if custom links is enabled.

Comment 3 by yyushkina@chromium.org, Sep 11

Labels: -M-68 -Target-69 -Target-71
Can you aim to fix this for 70 and merge in the fix?
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 11 

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

commit 68c436ef0f77da21093ae5df4cabf473ddc73a24
Author: kristipark <kristipark@chromium.org>
Date: Tue Sep 11 03:17:25 2018

[NTP] Fix the Most Visited notification for third party NTPs

The new pill notification functions for hide/show were being called
instead of the old notification functions.

Bug:  881781 
Change-Id: Ibc6012a1a6ef23b1405d1121941c511a58910821
Reviewed-on: https://chromium-review.googlesource.com/1217595
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590184}
[modify] https://crrev.com/68c436ef0f77da21093ae5df4cabf473ddc73a24/chrome/browser/resources/local_ntp/local_ntp.js

Comment 5 by twelling...@chromium.org, Sep 11

Labels: zine-triaged

Comment 6 by avsha...@etouch.net, Sep 12

Labels: TE-Verified-M71 TE-Verified-71.0.3550.0
Update :
---------
Tested above issue in Canary build #71.0.3550.0 on Windows(7, 8, 8.1, 10), Linux(14.04 LTS) & Mac(10.12.6, 10.13.1, 10.14, 10.13.6) OS and the issue is fixed. 
Now, 'X' icon works as expected on Yahoo NTP, hence adding TE-Verified labels. Kindly review an attached screen-cast for reference.

Thank you..!
Canary_behaviour.mp4
612 KB View Download

Comment 8 by kristip...@chromium.org, Sep 13

Status: Fixed (was: Assigned)

Comment 9 by kristip...@chromium.org, Sep 14

Labels: Merge-Request-70
Verified the fix in c7 in the latest Canary (71.0.3552.2). Requesting merge to M70 for both 1217595 and 1220480.
Project Member

Comment 10 by sheriffbot@chromium.org, Sep 14

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 11 by abdulsyed@chromium.org, Sep 14

Labels: -Merge-Review-70 Merge-Approved-70
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 14

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f2e3d7c3b421e65179bee2a6d8e0da5c9260ef1f

commit f2e3d7c3b421e65179bee2a6d8e0da5c9260ef1f
Author: kristipark <kristipark@chromium.org>
Date: Fri Sep 14 18:32:12 2018

[Merge M70] [NTP] Fix the Most Visited notification for third party NTPs

The new pill notification functions for hide/show were being called
instead of the old notification functions.

Bug:  881781 
Change-Id: Ibc6012a1a6ef23b1405d1121941c511a58910821
Reviewed-on: https://chromium-review.googlesource.com/1217595
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#590184}(cherry picked from commit 68c436ef0f77da21093ae5df4cabf473ddc73a24)
Reviewed-on: https://chromium-review.googlesource.com/1227175
Reviewed-by: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#412}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/f2e3d7c3b421e65179bee2a6d8e0da5c9260ef1f/chrome/browser/resources/local_ntp/local_ntp.js
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 14 

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

commit ae2e25d15cf1d4e072eb74ea6292c653f7ccc18a
Author: kristipark <kristipark@chromium.org>
Date: Fri Sep 14 18:33:53 2018

[Merge M70] [NTP] Fix custom links for third-party NTPs

Do not show custom links for third-party NTPs. Instead, return the
regular Most Visited tiles.

Bug:  881781 
Change-Id: Ifd2f587bd60d6ca2a54dcb262874c2b292bf24af
Reviewed-on: https://chromium-review.googlesource.com/1220480
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#591072}(cherry picked from commit f8fa4133861ab567647f6ccb320195628761793d)
Reviewed-on: https://chromium-review.googlesource.com/1227176
Reviewed-by: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#413}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/ae2e25d15cf1d4e072eb74ea6292c653f7ccc18a/chrome/browser/search/instant_service.cc
[modify] https://crrev.com/ae2e25d15cf1d4e072eb74ea6292c653f7ccc18a/chrome/browser/search/instant_service.h
[modify] https://crrev.com/ae2e25d15cf1d4e072eb74ea6292c653f7ccc18a/components/ntp_tiles/most_visited_sites.cc
[modify] https://crrev.com/ae2e25d15cf1d4e072eb74ea6292c653f7ccc18a/components/ntp_tiles/most_visited_sites.h
[modify] https://crrev.com/ae2e25d15cf1d4e072eb74ea6292c653f7ccc18a/components/ntp_tiles/most_visited_sites_unittest.cc
►

Sign in to add a comment