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

Issue 895569 link

Starred by 1 user
Status: Fixed
Owner:
█ freedjm@chromium.org
Last visit > 30 days ago
Closed: Nov 30
Cc:
benmason@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Explore Sites composite category images on NTP are missing favicons

Project Member Reported by freedjm@chromium.org, Oct 15 
For the composite category images on the NTP, we simply grab the first four sites for the category from the catalog. When some of these sites are missing a favicon in the database it causes the composite to not include a favicon for that site. The result is that we often see composite category images with less than 4 icons, when there are actually more than four icons in the category. 

To fix this, we can simply not include sites without a favicon when we retrieve them from the database. So all retrieved sites will have a favicon.

It was fixed in https://chromium-review.googlesource.com/c/chromium/src/+/1279155 and needs to be merged into M71.

Comment 1 by freedjm@chromium.org, Oct 15

Labels: Merge-Request-71
Status: Fixed (was: Started)
Project Member

Comment 2 by sheriffbot@chromium.org, Oct 16

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Comment 3 by sheriffbot@chromium.org, Oct 22 

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
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 26 

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

Comment 5 by freedjm@chromium.org, Oct 26

Labels: Disable-Nags

Comment 6 by benmason@chromium.org, Oct 30

Cc: benmason@chromium.org

Comment 7 by benmason@chromium.org, Nov 26 

Please merge ASAP. Thanks.

Comment 8 by benmason@chromium.org, Nov 30

Status: Started (was: Fixed)
Please merge to 71, branch 3578 ASAP.

Comment 9 by benmason@chromium.org, Nov 30

Status: Fixed (was: Started)
This is merged now.

Comment 10 by gov...@chromium.org, Dec 1

Labels: -Merge-Approved-71 merge-merged-3578
Removing "Merge-Approved-71" and applying "merge-merged-3578" label per chat with benmason@ and comment #9.
►

Sign in to add a comment