New issue
Advanced search Search tips

Issue 722770 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

Favicon are different between the NTP and the ContentSuggestions surface

Project Member Reported by gambard@chromium.org, May 16 2017

Issue description

The favicon used in ContentSuggestions are not the same of the one used in the NTP. The only difference between the two, is that the ContentSuggestions allows smaller favicon.
The service exposing the favicon does not seem to return the biggest favicon possible.
 
I don't understand what the bug is

HistoryBackend::GetLargestFaviconForURL() returns the largest icon of type <link rel="icon"> which is larger than the minimum size. If there is no match, it returns the largest icon of type <link rel="apple-touch-icon">? which is larger than the minimum size
Cc: pkotw...@chromium.org
The problem is: I am accepting small favicon. But I want the favicon with a size as close as possible from the desired size.
The current code will return the first favicon bigger than the minimal size even if it is much smaller than the desired size.

So the idea is to change the code to return either the first favicon bigger than the desired size, or the biggest favicon for all possible type.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 9 2017

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

commit d8572898d2d9bab32c5888e87d1e779cd8d4c291
Author: gambard <gambard@chromium.org>
Date: Fri Jun 09 08:19:14 2017

LargeIconService returns icon of the desired size

For now LargeIconService returns the an icon bigger than the minimal size.
If the page has multiple type of favicon, it might not be the biggest favicon,
and it might be smaller than the desired size.
This CL changes the service to return an icon bigger than the desired size and
the minimum size, or the biggest possible.

BUG= 722770 

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

[modify] https://crrev.com/d8572898d2d9bab32c5888e87d1e779cd8d4c291/components/favicon/core/large_icon_service.cc
[modify] https://crrev.com/d8572898d2d9bab32c5888e87d1e779cd8d4c291/components/favicon/core/large_icon_service.h

Status: Fixed (was: Assigned)

Sign in to add a comment