Issue metadata
Sign in to add a comment
|
Regression: UNDO action is not performed for invalid URL on NTP.
Reported by
aiman.an...@etouch.net,
Oct 17
|
||||||||||||||||||||||
Issue descriptionChrome Version: 72.0.3583.0 (Official Build) Revision 6d94d291fef5dc66b71e3456371e2722c07f945b-refs/branch-heads/3583@{#1} (32/64 Bit) OS: Win(7,8,8.1,10). Pre-condition: Enable 'Enable using the Google local NTP' and 'New Tab Page Custom Links' under chrome://flags and relaunch the browser. What steps will reproduce the problem? 1. In chrome, open NTP and add an invalid URL. 2. Now click on UNDO and observe. Actual Result: UNDO action is not performed for invalid URL. Expected Result: Should be able to perform UNDO for added invalid URL. This is a regression issue, broken in M-71, and will soon update other info: Good Build:71.0.3575.0 (Revision:597883) Bad Build: 71.0.3576.0 (Revision:598282) Note: 1. Issue is not on Linux(14.04 LTS) and Mac(10.13.1, 10.13.6, 10.14.1) OS. 2. Unable to reproduce the issue on today's Dev build #71.0.3578.10, as Browser crash is observed on adding NTP shortcut(Bug id: 894742). 3. Hence, raising it on latest Canary #72.0.3583.0. Kindly refer attached screen-cast for reference. Thank You!
,
Oct 17
Update: Correction in Chrome Version: Chrome Version: 71.0.3578.10 (Official Build) a04d4e218f0813212aa325e038e8961b62fd40ff-refs/branch-heads/3578@{#68} (32/64 Bit). Correction in Note: 1. Able to reproduce the issue on today's Dev build #71.0.3578.10 and Canary build #72.0.3583.0 Kindly refer the attached screen-cast for reference. Apologizes!
,
Oct 17
Adding release blocker label for this issue.Please reduce priority or remove if not the case. Thank You!
,
Oct 17
,
Oct 17
,
Oct 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/adf056599a25e3d59d1272de431aae24b549f927 commit adf056599a25e3d59d1272de431aae24b549f927 Author: Kristi Park <kristipark@chromium.org> Date: Fri Oct 19 00:24:36 2018 [NTP] Separate internal custom link updates from user changes When a default URL scheme is updated after URL validation, it is counted as a user action. This causes "Undo" to revert the URL scheme update instead of the user's add/update action. Add an additional parameter to CustomLinksManager::UpdateLink in order to distinguish between internal and user changes. Internal changes will not update the previous state that is restored when CustomLinksManager::UndoAction is called. Bug: 896143 Change-Id: I643679fab5e546fac7e41214af6ff8a4ba9c8b53 Reviewed-on: https://chromium-review.googlesource.com/c/1287259 Commit-Queue: Kristi Park <kristipark@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#600988} [modify] https://crrev.com/adf056599a25e3d59d1272de431aae24b549f927/chrome/browser/search/instant_service.cc [modify] https://crrev.com/adf056599a25e3d59d1272de431aae24b549f927/components/ntp_tiles/custom_links_manager.h [modify] https://crrev.com/adf056599a25e3d59d1272de431aae24b549f927/components/ntp_tiles/custom_links_manager_impl.cc [modify] https://crrev.com/adf056599a25e3d59d1272de431aae24b549f927/components/ntp_tiles/custom_links_manager_impl.h [modify] https://crrev.com/adf056599a25e3d59d1272de431aae24b549f927/components/ntp_tiles/custom_links_manager_impl_unittest.cc [modify] https://crrev.com/adf056599a25e3d59d1272de431aae24b549f927/components/ntp_tiles/most_visited_sites.cc [modify] https://crrev.com/adf056599a25e3d59d1272de431aae24b549f927/components/ntp_tiles/most_visited_sites.h [modify] https://crrev.com/adf056599a25e3d59d1272de431aae24b549f927/components/ntp_tiles/most_visited_sites_unittest.cc
,
Oct 19
,
Oct 19
[Auto-generated comment by a script] We noticed that this issue is targeted for M-71; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-71 label, otherwise remove Merge-TBD label. Thanks.
,
Oct 19
Pls update bug with canary result tomorrow and request a merge to M71 if cl is safe to merge. Thank you.
,
Oct 19
The NextAction date has arrived: 2018-10-19
,
Oct 19
How is the change looking canary?
,
Oct 19
Change looks good in canary 72.0.3585.0. Requesting merge to M71.
,
Oct 19
Approving merge to M71 branch 3578 based on comment #12. Pls merge your change to M71 branch #3578 ASAP so we can pick it up for next M71 Beta release. Thank you.
,
Oct 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d3c9eab9454f747ffde304c218f081c51c6e6077 commit d3c9eab9454f747ffde304c218f081c51c6e6077 Author: Kristi Park <kristipark@chromium.org> Date: Fri Oct 19 22:47:40 2018 [Merge M71] [NTP] Separate internal custom link updates from user changes Merge: Resolved some conflicts in most_visited_sites.* due to https://chromium-review.googlesource.com/c/chromium/src/+/1272137 that being in M71. When a default URL scheme is updated after URL validation, it is counted as a user action. This causes "Undo" to revert the URL scheme update instead of the user's add/update action. Add an additional parameter to CustomLinksManager::UpdateLink in order to distinguish between internal and user changes. Internal changes will not update the previous state that is restored when CustomLinksManager::UndoAction is called. (cherry picked from commit adf056599a25e3d59d1272de431aae24b549f927) Bug: 896143 Change-Id: I643679fab5e546fac7e41214af6ff8a4ba9c8b53 Reviewed-on: https://chromium-review.googlesource.com/c/1287259 Commit-Queue: Kristi Park <kristipark@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#600988} Reviewed-on: https://chromium-review.googlesource.com/c/1292395 Reviewed-by: Kristi Park <kristipark@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#176} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/d3c9eab9454f747ffde304c218f081c51c6e6077/chrome/browser/search/instant_service.cc [modify] https://crrev.com/d3c9eab9454f747ffde304c218f081c51c6e6077/components/ntp_tiles/custom_links_manager.h [modify] https://crrev.com/d3c9eab9454f747ffde304c218f081c51c6e6077/components/ntp_tiles/custom_links_manager_impl.cc [modify] https://crrev.com/d3c9eab9454f747ffde304c218f081c51c6e6077/components/ntp_tiles/custom_links_manager_impl.h [modify] https://crrev.com/d3c9eab9454f747ffde304c218f081c51c6e6077/components/ntp_tiles/custom_links_manager_impl_unittest.cc [modify] https://crrev.com/d3c9eab9454f747ffde304c218f081c51c6e6077/components/ntp_tiles/most_visited_sites.cc [modify] https://crrev.com/d3c9eab9454f747ffde304c218f081c51c6e6077/components/ntp_tiles/most_visited_sites.h [modify] https://crrev.com/d3c9eab9454f747ffde304c218f081c51c6e6077/components/ntp_tiles/most_visited_sites_unittest.cc
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d3c9eab9454f747ffde304c218f081c51c6e6077 Commit: d3c9eab9454f747ffde304c218f081c51c6e6077 Author: kristipark@chromium.org Commiter: kristipark@chromium.org Date: 2018-10-19 22:47:40 +0000 UTC [Merge M71] [NTP] Separate internal custom link updates from user changes Merge: Resolved some conflicts in most_visited_sites.* due to https://chromium-review.googlesource.com/c/chromium/src/+/1272137 that being in M71. When a default URL scheme is updated after URL validation, it is counted as a user action. This causes "Undo" to revert the URL scheme update instead of the user's add/update action. Add an additional parameter to CustomLinksManager::UpdateLink in order to distinguish between internal and user changes. Internal changes will not update the previous state that is restored when CustomLinksManager::UndoAction is called. (cherry picked from commit adf056599a25e3d59d1272de431aae24b549f927) Bug: 896143 Change-Id: I643679fab5e546fac7e41214af6ff8a4ba9c8b53 Reviewed-on: https://chromium-review.googlesource.com/c/1287259 Commit-Queue: Kristi Park <kristipark@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#600988} Reviewed-on: https://chromium-review.googlesource.com/c/1292395 Reviewed-by: Kristi Park <kristipark@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#176} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6d2d998845985e6eb7fe3f25817782b247c91130 commit 6d2d998845985e6eb7fe3f25817782b247c91130 Author: Kristi Park <kristipark@chromium.org> Date: Thu Oct 25 00:16:58 2018 Revert "[NTP] Separate internal custom link updates from user changes" This reverts commit adf056599a25e3d59d1272de431aae24b549f927. Reason for revert: Removing the HEAD request. See https://crbug/874194. Original change's description: > [NTP] Separate internal custom link updates from user changes > > When a default URL scheme is updated after URL validation, it is counted > as a user action. This causes "Undo" to revert the URL scheme update > instead of the user's add/update action. > > Add an additional parameter to CustomLinksManager::UpdateLink in order > to distinguish between internal and user changes. Internal changes will > not update the previous state that is restored when > CustomLinksManager::UndoAction is called. > > Bug: 896143 > Change-Id: I643679fab5e546fac7e41214af6ff8a4ba9c8b53 > Reviewed-on: https://chromium-review.googlesource.com/c/1287259 > Commit-Queue: Kristi Park <kristipark@chromium.org> > Reviewed-by: Marc Treib <treib@chromium.org> > Cr-Commit-Position: refs/heads/master@{#600988} TBR=treib@chromium.org,kristipark@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 896143 Change-Id: I26fd1f3a92161fe06009681ff90b5ea65c2db2b7 Reviewed-on: https://chromium-review.googlesource.com/c/1298553 Reviewed-by: Kristi Park <kristipark@chromium.org> Commit-Queue: Kristi Park <kristipark@chromium.org> Cr-Commit-Position: refs/heads/master@{#602536} [modify] https://crrev.com/6d2d998845985e6eb7fe3f25817782b247c91130/chrome/browser/search/instant_service.cc [modify] https://crrev.com/6d2d998845985e6eb7fe3f25817782b247c91130/components/ntp_tiles/custom_links_manager.h [modify] https://crrev.com/6d2d998845985e6eb7fe3f25817782b247c91130/components/ntp_tiles/custom_links_manager_impl.cc [modify] https://crrev.com/6d2d998845985e6eb7fe3f25817782b247c91130/components/ntp_tiles/custom_links_manager_impl.h [modify] https://crrev.com/6d2d998845985e6eb7fe3f25817782b247c91130/components/ntp_tiles/custom_links_manager_impl_unittest.cc [modify] https://crrev.com/6d2d998845985e6eb7fe3f25817782b247c91130/components/ntp_tiles/most_visited_sites.cc [modify] https://crrev.com/6d2d998845985e6eb7fe3f25817782b247c91130/components/ntp_tiles/most_visited_sites.h [modify] https://crrev.com/6d2d998845985e6eb7fe3f25817782b247c91130/components/ntp_tiles/most_visited_sites_unittest.cc |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by aiman.an...@etouch.net
, Oct 17Status: Assigned (was: Unconfirmed)