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

Issue 668023 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : Favicon icon of a page is not seen properly after zooming in the chrome://bookmarks page.

Reported by yfulgaon...@etouch.net, Nov 23 2016

Issue description

Chrome Version : 57.0.2928.0 (Official Build) 61ab768cc656a8bef5e8f08972d0f58a74f89c83-refs/heads/master@{#433845} 32/64 bit
OS : Mac(10.11.6, 10.12.1, 10.12), Windows(7,8,8.1,10), Linux(14.04 LTS)

What steps will reproduce the problem?
1. Launch chrome and navigate to chrome://bookmarks/ page.
2. From ‘Organise’ menu, add a new page.
3. Now hit ‘Cmd ++’ to zoom in the page and observe the favicon icon of newly added page.

Actual : Favicon icon of a page is not seen properly after zooming in the chrome://bookmarks page.
Expected : Favicon icon of a page should be seen properly after zooming in the bookmarks page.

This is a regression issue broken in ‘M-55’, below is the Manual Regression range and will soon update other info.
Good build : 55.0.2867.0
Bad build : 55.0.2868.0
 
Favicon_icon.png
57.0 KB View Download
Actual_favicon_icon.mov
5.5 MB Download
Expected_favicon_icon.mov
5.0 MB Download
Labels: hasbisect-per-revision
Owner: pkasting@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 55.0.2867.0 (Revision: 419930).
Bad build: 55.0.2868.0 (Revision: 420217).

You are probably looking for a change made after 419987 (known good), but no later than 419988 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/05d38461379b7a86bc48ef345afd1fc835a00c37..71fa2e9ca7722388da15099748e2772f9f1640e7

@pkasting -- Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Thank You.
* Only breaks the "default favicon" icon, not folder icons or specific favicons
* The icon does zoom, it just seems to choose the wrong base size at >1x, so it's 50% too small
I did some investigation on the issue, and have some findings. The root cause, actually is not this commit (https://chromium.googlesource.com/chromium/src/+/71fa2e9ca7722388da15099748e2772f9f1640e7), but it triggers.

I notice that we are getting the default favicon size in DIP, please see https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/favicon_source.cc?q=FaviconSource&sq=package:chromium&dr=CSs&l=183, which will behave wrong when the page zooms in. By using the STABLE release, you can test this by zooming in the page chrome://bookmarks, the default favicon becomes blurred. The reason why this issue doesn't appear before is that we defined the width to 16px.

I have uploaded a CL (https://codereview.chromium.org/2535463002/), thanks!
Just to update latest behavior, still this issue is observed on Ubuntu 14.04 uisng chrome latest Dev M57-57.0.2938.0.

Could anyone let us know is there any latest update available on this issue? 
The fix is in progress, you can look at the CL link in comment 3 for details.
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 5 2016

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

commit 47fcdcbb580f9077c0b484ffc43a01f71960e06a
Author: minggang.wang <minggang.wang@intel.com>
Date: Mon Dec 05 07:11:58 2016

Currently, we are caching the default favicon, but the scale factor is
ignored. So we need to remove this in order to take the required scale
factor into account.

BUG= 668023 

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

[modify] https://crrev.com/47fcdcbb580f9077c0b484ffc43a01f71960e06a/AUTHORS
[modify] https://crrev.com/47fcdcbb580f9077c0b484ffc43a01f71960e06a/chrome/browser/ui/webui/favicon_source.cc
[modify] https://crrev.com/47fcdcbb580f9077c0b484ffc43a01f71960e06a/chrome/browser/ui/webui/favicon_source.h

Status: Fixed (was: Assigned)
Assuming fixed by r436237.

Sign in to add a comment