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

Issue 702233 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: ----



Sign in to add a comment

Incorrect icon for Web apps

Project Member Reported by zpeng@chromium.org, Mar 16 2017

Issue description

Example:
sensor-compass.appspot.com

Icon is correct on M56 (stable) & M57 (beta).
Icon is incorrect on M58 (dev) & M59 (canary).
 

Comment 1 by zpeng@chromium.org, Mar 16 2017

Labels: M-58

Comment 2 by zpeng@chromium.org, Mar 16 2017

The correct icon is a compass, the incorrect icon shown and added to the home screen is an "A" on a gray background.

Below is the testing results on different devices:
Device         M56(Stable)  M57(Beta)  M58(Dev)  M59(Canary)  ToT 
Galaxy S5 (M)  Correct      Correct    Correct   Correct      Correct
Nexus 5X (N)   Correct      Correct    Incorrect Incorrect    Incorrect
Nexus 6P (N)   Correct      Correct    Incorrect Incorrect    Correct

Comment 3 by zpeng@chromium.org, Mar 17 2017

Owner: zpeng@chromium.org
Status: Started (was: Available)
Similar issue found for superbowl.com

Comment 4 by zpeng@chromium.org, Mar 30 2017

Cc: dominickn@chromium.org
Labels: -Pri-1 -M-58 Pri-2
Waiting on PM's input on whether we want to use the web manifest's icon when the websites do not pass the installability checks.

Cc: sbirch@chromium.org owe...@chromium.org
+owencm@, +sbirch@

The issue here is that if a site has a manifest, but it doesn't qualify as being installable (e.g. doesn't have display: standalone, icons are too small, no service worker), we skip fetching the manifest icon entirely when the user taps add to homescreen.

This means that there are some sites that specify a manifest and icons in it, but they're not installable, so we don't consider their icons. This is an explicit optimisation right now in the installability pipeline.

What do you think about moving the icon fetch before the installability check? It means that for these sites, we'll typically use more data (we fetch the icon before bailing out of the checking). However, it means that for add to homescreen, these sites will get a shortcut with a better icon even though they don't qualify for a WebAPK.
Can we fetch the icon only when the user subsequently adds the shortcut? IIUC, then we would only use more data when that data would be useful and they'd get a nice icon.
No, we have to fetch the icon upfront when the user taps add to homescreen and show the user in the A2HS dialog. Otherwise the user won't know what the icon they're adding to their homescreen will look like.
Spoke with sbirch offline. He's in favour of fetching the icon always, so let's roll with that. :)
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 8 2017

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

commit f7338b32ab404c8dca728c4b8dabb8b660d5b4ec
Author: zpeng <zpeng@chromium.org>
Date: Sat Apr 08 15:11:08 2017

Fetch manifest icon prior to checking eligibility in InstallableManager

Previously primary icons are not fetched for web sites that do not pass
the installability tests. As a result, web sites such as
sensor-compass.appspot.com and superbowl.com, when added to home
screen, have generated icons instead of primary icons provided in their
Web manifests.

This CL makes InstallableManager to fetch manifest icon prior to
checking eligibility for PWA in InstallableManager.

BUG= 702233 

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

[modify] https://crrev.com/f7338b32ab404c8dca728c4b8dabb8b660d5b4ec/chrome/browser/installable/installable_manager.cc
[modify] https://crrev.com/f7338b32ab404c8dca728c4b8dabb8b660d5b4ec/chrome/browser/installable/installable_manager_browsertest.cc

Comment 10 by zpeng@chromium.org, Apr 20 2017

Status: Fixed (was: Started)

Sign in to add a comment