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

Issue 819076 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature



Sign in to add a comment

Tab should update to better favicon while ideal size not received

Reported by shu...@amazon.com, Mar 6 2018

Issue description

Steps to reproduce the problem:
1. Suppose "default_favicon_size" (in src/chrome/android/java/res/values/dimens.xml) is set to 16dp and the scale factor is 1x.
2. Suppose a website, say example.com, serves favicons which are 8px by 8px and 32px by 32px.
3. If the first favicon served by the website is 8x8, then the Tab will never update to start using the better 32x32 favicon.

To be able to repro this bug easily, either increase the value of "default_favicon_size" (src/chrome/android/java/res/values/dimens.xml) to a higher value, or choose a device with a high pixel density.

What is the expected behavior?

What went wrong?
If a website serves more than one favicon, the Tab starts displaying the
one it receives first. It only updates the favicon if a subsequent one
is exactly the size defined by |mIdealFaviconSize|. This is a problem
when the initially served favicon is smaller than |mIdealFaviconSize|,
and subsequent ones are greater. In such cases, the Tab continues to use
a lower res favicon even though higher resolution ones are available.

Ideally, the favicon should be updated until one which is at least 
|mIdealFaviconSize| is received (with an exact match overriding even a
larger one).

Did this work before? N/A 

Chrome version: master  Channel: n/a
OS Version: 6.0.1
Flash Version:
 
Cc: twelling...@chromium.org
Labels: Needs-triage-Mobile Triaged-Mobile
@shubag: Thanks for the report!!

CC'ing reviewer from https://chromium-review.googlesource.com/#/c/chromium/src/+/950473 for further update on this issue.

@twellington: Could you please take a look at this issue and help?

Thanks!!
Gentle ping. 
@twellington -- could you please look into the issue as requested in C#2.

Thanks!
Cc: bauerb@chromium.org
Components: -UI UI>Browser>Mobile
Labels: -Type-Bug Type-Feature
Status: Available (was: Unconfirmed)
+bauerb@ has been the most active reviewer on the CL mentioned in #2

I think this is an "as time allows" feature request.

Comment 5 by bauerb@chromium.org, May 15 2018

Yes, I don't think this bug needs action from us right now. 
Project Member

Comment 6 by bugdroid1@chromium.org, May 22 2018

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

commit 612456881e8484ea3926f44e7c6f59af6c0c570f
Author: Shubham Agrawal <shubag@amazon.com>
Date: Tue May 22 12:50:06 2018

Tab should update to better favicon while ideal size not received

If a website serves more than one favicon, the Tab starts displaying the
one it receives first. It only updates this favicon if a subsequent one
is exactly the size defined by |mIdealFaviconSize|. This is a problem
when the initially served favicon is smaller than |mIdealFaviconSize|,
and subsequent ones are larger. In such cases, the Tab continues to use
a lower res favicon even though higher resolution ones are available.

With this change, the favicon is updated until one that is at least as
large as |mIdealFaviconSize| is received, with an exact match overriding
even a larger one.

TEST=manually verified that the favicon is updated as expected (by
changing default_favicon_size to a higher value)
BUG=819076

Change-Id: I47c9266180c787b65a480dab228c7c5ea43dfd96
Reviewed-on: https://chromium-review.googlesource.com/950473
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560545}
[modify] https://crrev.com/612456881e8484ea3926f44e7c6f59af6c0c570f/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreference.java
[modify] https://crrev.com/612456881e8484ea3926f44e7c6f59af6c0c570f/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java

Sign in to add a comment