incognito bookmarks shouldn't appear on 'recent bookmarks'
Reported by
jleedev@gmail.com,
Nov 4 2016
|
|||||||||
Issue descriptionSteps to reproduce the problem: 1. In Incognito, browse to a page that you have bookmarked, or open the bookmark specifically 2. Open regular tab and see your browsing history right there What is the expected behavior? Recent bookmarks should exclude page visits that occurred in incognito mode. If possible, it should exclude bookmarks that were created from an incognito window. Obviously, the fact that a page is bookmarked cannot be discarded upon exiting incognito mode, but "This page was visited at this time" seems as close to browsing history as you can get. This gives the user the impression that bookmarking a page exempts it from the promise Chrome makes when you open an incognito window, which is confusing. What went wrong? The "Recent bookmarks" section in the new tab page shows bookmarked pages that were visited in incognito (and when!). Did this work before? N/A Chrome version: 54.0.2840.68 Channel: stable OS Version: Android 7.0.0; Nexus 5X Build/NBD90W Flash Version: I am quite amused to reopen, nearly word for word, a seven-year-old bug with five-digit id number (15392). The resolution of that bug was "We reimplemented the new tab page", which happened a dozen times since and has wrapped around back to where it started.
,
Dec 12 2016
,
Dec 12 2016
Thanks for the report!
,
Dec 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1e385fa1c19d96f343594a0623a7985adb50c483 commit 1e385fa1c19d96f343594a0623a7985adb50c483 Author: jkrcal <jkrcal@chromium.org> Date: Thu Dec 15 16:22:56 2016 Disable tracking of visits to bookmarks in incognito Date of the last visit to any bookmarked page is stored to BookmarkModel. Previously, this happens in off-the-record profiles as well. This CL fixes the issue. BUG= 662382 Review-Url: https://codereview.chromium.org/2572433002 Cr-Commit-Position: refs/heads/master@{#438845} [modify] https://crrev.com/1e385fa1c19d96f343594a0623a7985adb50c483/chrome/browser/ntp_snippets/bookmark_last_visit_updater.cc [modify] https://crrev.com/1e385fa1c19d96f343594a0623a7985adb50c483/chrome/browser/ntp_snippets/bookmark_last_visit_updater.h [add] https://crrev.com/1e385fa1c19d96f343594a0623a7985adb50c483/chrome/browser/ntp_snippets/bookmark_last_visit_updater_unittest.cc [modify] https://crrev.com/1e385fa1c19d96f343594a0623a7985adb50c483/chrome/browser/ui/tab_helpers.cc [modify] https://crrev.com/1e385fa1c19d96f343594a0623a7985adb50c483/chrome/test/BUILD.gn
,
Dec 19 2016
Still need to verify it in the last canary and merge to M56.
,
Dec 22 2016
Verified on 57.0.2957.0. I would like to merge to M56 because this is a privacy-sensitive bug. The CL is relatively small.
,
Dec 22 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 26 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 29 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/62f09cadded58c144c804dfacf0ef7305247ad90 commit 62f09cadded58c144c804dfacf0ef7305247ad90 Author: Jan Krcal <jkrcal@chromium.org> Date: Mon Jan 02 11:45:55 2017 Disable tracking of visits to bookmarks in incognito Date of the last visit to any bookmarked page is stored to BookmarkModel. Previously, this happens in off-the-record profiles as well. This CL fixes the issue. BUG= 662382 Review-Url: https://codereview.chromium.org/2572433002 Cr-Commit-Position: refs/heads/master@{#438845} (cherry picked from commit 1e385fa1c19d96f343594a0623a7985adb50c483) Review-Url: https://codereview.chromium.org/2603213002 . Cr-Commit-Position: refs/branch-heads/2924@{#641} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/62f09cadded58c144c804dfacf0ef7305247ad90/chrome/browser/ntp_snippets/bookmark_last_visit_updater.cc [modify] https://crrev.com/62f09cadded58c144c804dfacf0ef7305247ad90/chrome/browser/ntp_snippets/bookmark_last_visit_updater.h [add] https://crrev.com/62f09cadded58c144c804dfacf0ef7305247ad90/chrome/browser/ntp_snippets/bookmark_last_visit_updater_unittest.cc [modify] https://crrev.com/62f09cadded58c144c804dfacf0ef7305247ad90/chrome/browser/ui/tab_helpers.cc [modify] https://crrev.com/62f09cadded58c144c804dfacf0ef7305247ad90/chrome/test/BUILD.gn
,
Jan 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/09a5dafdb83912be2c0033c29fde7e641c5118c8 commit 09a5dafdb83912be2c0033c29fde7e641c5118c8 Author: jkrcal <jkrcal@chromium.org> Date: Tue Jan 03 09:23:43 2017 Revert of Disable tracking of visits to bookmarks in incognito (patchset #1 id:1 of https://codereview.chromium.org/2603213002/ ) Reason for revert: bookmark_last_visit_updater_unittest.cc does not compile on the M56 branch. Signature of one function that I use in the unit-test is different from the one on trunk, I need to fix it. Original issue's description: > Disable tracking of visits to bookmarks in incognito > > Date of the last visit to any bookmarked page is stored to > BookmarkModel. Previously, this happens in off-the-record profiles as > well. This CL fixes the issue. > > BUG= 662382 > > Had to manually resolve (trivial) conflicts within drover when merging to M56, so I add TBR. > > TBR=avi@chromium.org,bauerb@chromium.org,battre@chromium.org > > Review-Url: https://codereview.chromium.org/2572433002 > Cr-Commit-Position: refs/heads/master@{#438845} > (cherry picked from commit 1e385fa1c19d96f343594a0623a7985adb50c483) > > Committed: https://chromium.googlesource.com/chromium/src/+/62f09cadded58c144c804dfacf0ef7305247ad90 TBR=avi@chromium.org,bauerb@chromium.org,battre@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 662382 Review-Url: https://codereview.chromium.org/2606283003 Cr-Commit-Position: refs/branch-heads/2924@{#643} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/09a5dafdb83912be2c0033c29fde7e641c5118c8/chrome/browser/ntp_snippets/bookmark_last_visit_updater.cc [modify] https://crrev.com/09a5dafdb83912be2c0033c29fde7e641c5118c8/chrome/browser/ntp_snippets/bookmark_last_visit_updater.h [delete] https://crrev.com/b5392712e6b107d76bcfb594ba0cfdf87c946b0e/chrome/browser/ntp_snippets/bookmark_last_visit_updater_unittest.cc [modify] https://crrev.com/09a5dafdb83912be2c0033c29fde7e641c5118c8/chrome/browser/ui/tab_helpers.cc [modify] https://crrev.com/09a5dafdb83912be2c0033c29fde7e641c5118c8/chrome/test/BUILD.gn
,
Jan 3 2017
I had to revert the merged CL, I started working on a fixed CL for the M56 branch.
,
Jan 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a3d2182e6981362bd8e66ca4f706922927429bb0 commit a3d2182e6981362bd8e66ca4f706922927429bb0 Author: jkrcal <jkrcal@chromium.org> Date: Tue Jan 03 16:43:58 2017 Disable tracking of visits to bookmarks in incognito Date of the last visit to any bookmarked page is stored to BookmarkModel. Previously, this happens in off-the-record profiles as well. This CL fixes the issue. Did not merge cleanly (with simple conflicts, though). Compared to the original CL, this also adapts to a difference in the M56 branch from trunk - ntp_snippets::GetRecentlyVisitedBookmarks() takes 6 arguments. BUG= 662382 , 677814 NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2572433002 Cr-Commit-Position: refs/heads/master@{#438845} (cherry picked from commit 1e385fa1c19d96f343594a0623a7985adb50c483) Review-Url: https://codereview.chromium.org/2603323002 Cr-Commit-Position: refs/branch-heads/2924@{#647} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/a3d2182e6981362bd8e66ca4f706922927429bb0/chrome/browser/ntp_snippets/bookmark_last_visit_updater.cc [modify] https://crrev.com/a3d2182e6981362bd8e66ca4f706922927429bb0/chrome/browser/ntp_snippets/bookmark_last_visit_updater.h [add] https://crrev.com/a3d2182e6981362bd8e66ca4f706922927429bb0/chrome/browser/ntp_snippets/bookmark_last_visit_updater_unittest.cc [modify] https://crrev.com/a3d2182e6981362bd8e66ca4f706922927429bb0/chrome/browser/ui/tab_helpers.cc [modify] https://crrev.com/a3d2182e6981362bd8e66ca4f706922927429bb0/chrome/test/BUILD.gn
,
Jan 4 2017
Verified in M56-56.0.2924.51
,
Jan 12 2017
+jkrcal -- we can mark this as fixed, right?
,
Jan 12 2017
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by kravula@google.com
, Nov 4 2016Owner: nepper@chromium.org
Status: Assigned (was: Unconfirmed)