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

Issue 874018 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Regression: Shortcut gets added even by clicking on cancel button on NTP

Reported by sanyam.g...@etouch.net, Aug 14

Issue description

Chrome Version: 70.0.3521.2 (Official Build) Revision	eace06098e75dd5b54de82024208123fc1a51350-refs/branch-heads/3521@{#5} (32/64-bit)
OS: Windows(7,8,8.1,10), Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14) and Linux(14.04)

Pre-condition: Enable 'Enable using the Google local NTP' and 'New Tab Page Custom Links' under chrome://flags.

What steps will reproduce the problem?
(1) Launch Chrome, navigate to NTP and click on 'Add shortcut' thumbnail.
(2) In 'Add shortcut' overlay, enter text in 'Name' & 'URL' fields.
(3) Now click on 'Cancel' button and observe.

Actual Result  : Shortcut gets added even by clicking on cancel button.
Expected Result: Shortcut should not get added by clicking on cancel button.

This is a regression issue, broken in 'M-70', and below is  per-revision bisect-info:
Good Build: 70.0.3514.0 (Revision:580791)
Bad Build : 70.0.3515.0 (Revision:581084)

You are probably looking for a change made after 580948 (known good), but no later than 580949 (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/631ac11aae294abc8b5a100a09e59af59aa6e229..c2111e0695751dcd6833f86930ea7905daef9494

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

@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.
 
Kindly review the attached screen-cast .

Thank You!
 
Actual_Behaviour.mp4
815 KB View Download
Expected_Behaviour.mp4
701 KB View Download
Cc: yyushkina@chromium.org
Labels: -Target-70 Target-69 FoundIn-69
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 14

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

commit 31f1f7d3c6a89e80c5bfe1895c187b8d3e22c499
Author: kristipark <kristipark@chromium.org>
Date: Tue Aug 14 19:58:26 2018

[NTP] Fix cancel button submitting when adding custom link

"Cancel" was calling the form's submit event, which triggered the add
link flow.

Bug:  874018 
Change-Id: Iad822d3884484f5dc0d9f3e6667c9d4c9e0a2728
Reviewed-on: https://chromium-review.googlesource.com/1174908
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583004}
[modify] https://crrev.com/31f1f7d3c6a89e80c5bfe1895c187b8d3e22c499/chrome/browser/resources/local_ntp/custom_links_edit.js

Status: Fixed (was: Started)
Status: Started (was: Fixed)
Closed prematurely. The fix bypassed the required fields in the form.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 15

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

commit ab2d8581e7e91e80fc7cc807e21992b94ee4fc3c
Author: Kristi Park <kristipark@chromium.org>
Date: Wed Aug 15 17:24:31 2018

Revert "[NTP] Fix cancel button submitting when adding custom link"

This reverts commit 31f1f7d3c6a89e80c5bfe1895c187b8d3e22c499.

Reason for revert: Bypasses the form's required fields when clicking
"Done"

Original change's description:
> [NTP] Fix cancel button submitting when adding custom link
> 
> "Cancel" was calling the form's submit event, which triggered the add
> link flow.
> 
> Bug:  874018 
> Change-Id: Iad822d3884484f5dc0d9f3e6667c9d4c9e0a2728
> Reviewed-on: https://chromium-review.googlesource.com/1174908
> Reviewed-by: Fernando Serboncini <fserb@chromium.org>
> Commit-Queue: Kristi Park <kristipark@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583004}

TBR=fserb@chromium.org,kristipark@chromium.org

Change-Id: Ie605d0a5d5a5043fbb76d30eb98b7565818e41f7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  874018 
Reviewed-on: https://chromium-review.googlesource.com/1174775
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583299}
[modify] https://crrev.com/ab2d8581e7e91e80fc7cc807e21992b94ee4fc3c/chrome/browser/resources/local_ntp/custom_links_edit.js

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 15

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

commit 2c54ca333586ba0172155a341e9ac3e81e514183
Author: Kristi Park <kristipark@chromium.org>
Date: Wed Aug 15 23:28:11 2018

[NTP] Prevent the Cancel and Delete buttons from submitting the edit custom link form

<button> is type="submit" by default. Adding type="button" will prevent
the Cancel and Delete buttons from triggering the "submit" event.

Bug:  874018 
Change-Id: Ie72ec07ddbaeccee29588612fc8938f268150f20
Reviewed-on: https://chromium-review.googlesource.com/1174804
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583441}
[modify] https://crrev.com/2c54ca333586ba0172155a341e9ac3e81e514183/chrome/browser/resources/local_ntp/custom_links_edit.html

Status: Fixed (was: Started)
Update : 
Retested above issue on Windows (7, 8, 8.1, 10), Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14) and Linux(14.04) OS using latest Canary #70.0.3524.0 and issue is still reproducible.
 
Kindly review the attached screen-cast.
Thank you
Canary_Behaviour.mp4
667 KB View Download
As per comment #7 fix not landed in latest canary #70.0.3524.0(#583420).Will check and update the bug once fix is available.

Thank You!
Labels: TE-Verified-M70 TE-Verified-70.0.3525.0
Update : 
Retested the above issue on Windows (7, 8, 8.1, 10), Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14) and Linux(14.04) OS using latest Canary #70.0.3525.0 and issue is fixed.
 
Kindly review the attached screen-cast.
Thank you
Fixed_Behaviour.mp4
548 KB View Download

Sign in to add a comment