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

Issue 638538 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Feature



Sign in to add a comment

No bookmarks (and no bookmark button) are shown after update to M54.

Project Member Reported by jkrcal@chromium.org, Aug 17 2016

Issue description

Users have no info about last visited dates for bookmarks when they update to M54.
Previously, we show bookmarks button on NTP. Now, we show nothing before the user visits any bookmark.

As a solution, introduce a special mode before we store any last visited dates for bookmarks. In this mode, 
 - creation data for bookmarks is considered,
 - no age limit is imposed,
 - only 3 bookmarks are shown (configurable by Finch).
 

Comment 1 by jkrcal@chromium.org, Aug 17 2016

Another solution after discussing with Marc:

Add a fallback to creation date. This fallback is active when we have less bookmarks with last visit info than specified by Finch (defaults to 3).

In this fallback mode, we
 - use creation date instead of (missing) last visit date and
 - do not impose age limit on bookmarks to show

Comment 2 by jkrcal@chromium.org, Aug 17 2016

This is addressed by CL 2256643002.

Comment 3 by jkrcal@chromium.org, Aug 17 2016

Cc: treib@chromium.org pke@google.com
Another feature request:

Have this fallback to creation_date active only for 6 weeks after the user runs M54 for the first time. This way, we deal properly with users (e.g. from EM) that update to M54 much later than it is released.

Comment 4 by treib@chromium.org, Aug 18 2016

Cc: jkrcal@chromium.org
Owner: ----
Status: Available (was: Assigned)
Un-assinging since Jan is OOO now. We'll see who picks it up :)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 18 2016

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

commit a14bce74b74b8a043608b485dda1cb65191b24da
Author: jkrcal <jkrcal@chromium.org>
Date: Thu Aug 18 10:04:08 2016

Add a fallback to creation date for Recent bookmarks on NTP

This fallback is active when we have less bookmarks with last visit info than specified by Finch (defaults to 3). This is relevant for existing users after they update to m54.

In this fallback mode, we
 - use creation date instead of (missing) last visit date and
 - do not impose age limit on bookmarks to show

BUG= 638538 

Review-Url: https://codereview.chromium.org/2256643002
Cr-Commit-Position: refs/heads/master@{#412782}

[modify] https://crrev.com/a14bce74b74b8a043608b485dda1cb65191b24da/components/ntp_snippets/bookmarks/bookmark_last_visit_utils.cc
[modify] https://crrev.com/a14bce74b74b8a043608b485dda1cb65191b24da/components/ntp_snippets/bookmarks/bookmark_last_visit_utils.h
[modify] https://crrev.com/a14bce74b74b8a043608b485dda1cb65191b24da/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc

Comment 6 by pke@google.com, Aug 19 2016

Labels: zine-16-08-15
Owner: pke@google.com
Status: Started (was: Available)

Comment 7 by fi...@chromium.org, Aug 19 2016

Labels: zine-triaged
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 19 2016

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

commit dc5e1eb66477e2efba986ce9dff6be91258709bd
Author: pke <pke@google.com>
Date: Fri Aug 19 18:34:59 2016

Use bookmark creation date fallback for 6 weeks after installing M54

Add pref that stores the time the BookmarkSuggestionsProvider was first
started on M54. Enable the creation_date_fallback only if that time is
at most 6 weeks in the past.

Fix minor nits from previous CL 2256643002/#ps20001.

BUG= 638538 

Review-Url: https://codereview.chromium.org/2256183004
Cr-Commit-Position: refs/heads/master@{#413193}

[modify] https://crrev.com/dc5e1eb66477e2efba986ce9dff6be91258709bd/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
[modify] https://crrev.com/dc5e1eb66477e2efba986ce9dff6be91258709bd/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/dc5e1eb66477e2efba986ce9dff6be91258709bd/components/ntp_snippets/bookmarks/bookmark_last_visit_utils.cc
[modify] https://crrev.com/dc5e1eb66477e2efba986ce9dff6be91258709bd/components/ntp_snippets/bookmarks/bookmark_last_visit_utils.h
[modify] https://crrev.com/dc5e1eb66477e2efba986ce9dff6be91258709bd/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc
[modify] https://crrev.com/dc5e1eb66477e2efba986ce9dff6be91258709bd/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.h
[modify] https://crrev.com/dc5e1eb66477e2efba986ce9dff6be91258709bd/components/ntp_snippets/pref_names.cc
[modify] https://crrev.com/dc5e1eb66477e2efba986ce9dff6be91258709bd/components/ntp_snippets/pref_names.h

Comment 9 by pke@google.com, Aug 20 2016

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 23 2016

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

commit ea3816100645db1ab7c5e85ea52b5e5230264502
Author: treib <treib@chromium.org>
Date: Tue Aug 23 17:15:50 2016

Bookmark Suggestions: Make 'creation date fallback' time configurable

BUG= 638538 

Review-Url: https://codereview.chromium.org/2275613002
Cr-Commit-Position: refs/heads/master@{#413766}

[modify] https://crrev.com/ea3816100645db1ab7c5e85ea52b5e5230264502/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc

Sign in to add a comment