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

Issue 595517 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

App install banner doesn't trigger on desktop when largest_icon_size < (scale_factor * 144)

Project Member Reported by owe...@chromium.org, Mar 17 2016

Issue description

In Chrome Canary (51.0.2680.0 , OSX) the app install banner is not triggering for this manifest:

{  
  "name": "Baca Berita Indonesia",  
  "short_name": "BaBe",
  "icons": [{  
    "src": "/public/images/homescreen.png",  
    "sizes": "144x144",
    "type": "image/png"
  }],
  "start_url": "/",  
  "display": "standalone",
  "orientation": "portrait",
  "theme_color"  : "#ff9800",
  "background_color": "#ffffff",
  "gcm_sender_id": "358814325326"
}

You can see this in production on https://app.babe.co.id

In the console I am seeing "App banner not shown: could not determine the best icon to use".

Dominick tells me this triggers for him currently so it sounds like a regression worth digging into.
 
Components: Platform>Extensions
I can reproduce this issue on 51.0.2680 Canary on OS X.

But I can't reproduce it on 51.0.2681.0 (HEAD) on Linux.

I also can't reproduce it on 51.0.2680 on Linux.

I'll follow up regarding HEAD on OS X....
I couldn't reproduce this issue on my Mac Pro. Which led me to realise: I can't reproduce this issue on non-Retina screens. It only reproduces on my Retina laptop.

You're running on a Retina Mac, correct? That makes this an image density issue. On desktop, the app banner code currently looks for a minimum icon size of 128 pixels. On a 2x scale factor, that's a 256 display pixel icon. But this PWA provides a 144px icon, which the size check.

I believe this app should work correctly on Android and non-retina desktop platforms. 144px has been the advertised icon size on mobile for some time. How do you want to address this?
Components: -Platform>Extensions UI>Browser>AppShortcuts
Summary: App install banner doesn't trigger on desktop when largest_icon_size < (scale_factor * 144) (was: App install banner not triggering on valid manifest)

Comment 5 by owe...@chromium.org, Mar 17 2016

Cc: paulkinlan@chromium.org
Great job thinking of that! Makes a lot of sense. Could be an annoying issue to debug for developers though if they have a 1x laptop and 2x phone and it's not showing up there.

Paul - what's your take here? Should we encourage sites to include a large icon by default to handle this case? Also while we're at it, is it specifically 144x144 which is required for 1x devices today or would something larger, e.g. 155x155 work?
This is currently only a problem on desktop, where we want a larger icon (there's actually a TODO to work out what the best size should be). On mobile, I believe homescreen shortcuts are 48dp; on a device with a 3x scale factor like a Nexus 5X, this translates to 144px and all is good. Though if a mobile device with a 4x scale factor comes along then we may be out of luck. 
Components: UI>HighDPI
Labels: OS-Linux OS-Windows
Labels: -Type-Bug-Regression Type-Bug
Labels: OS-Chrome
144 has only been advertised as the minimum requirement, device icons on Android ideally need to be 192px x 192px and that is what we are recommending.  My understanding is that the logic for homescreen icons is based on platform and every platform is different and we need to make sure developers are aware of what icons to make. The concern in the past is that if we say make a 1024px image then everyone is wasting data and this is an important issue on mobile.

re:155, the logic is here https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/manifest/manifest_icon_downloader.cc&sq=package:chromium&type=cs&l=152&rcl=1458603371 - the logic tries to find the image with the closest dimensions to the ideal size for density.

I would expect the manifest checking in devtools to be mapped to the device density in the emulation settings in devtools and then in the error message to say "expecting 192px image"
Status: Assigned (was: Untriaged)
I'll put up a CL to improve the debug messaging in this case, explicitly calling out the icon size requirement.
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 25 2016

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

commit bf032db305e5391644d614cc151a6599f8b0fbf6
Author: dominickn <dominickn@chromium.org>
Date: Fri Mar 25 00:06:48 2016

Log the expected icon size when no suitable icon for a banner is found.

This CL improves the debug messaging for developers when an app banner
cannot be displayed due to an icon which is too small. The existing
messaging ("no suitable icon can be found") is split into two cases:
when all available icons are too small, and when the selected icon
cannot be fetched. In the former case, the exact required icon size
(respecting the device scale factor, which means a different sized icon
may be required on different platforms) is printed to console to better
inform developers of the icon requirements.

BUG= 595517 

Review URL: https://codereview.chromium.org/1826753002

Cr-Commit-Position: refs/heads/master@{#383196}

[modify] https://crrev.com/bf032db305e5391644d614cc151a6599f8b0fbf6/chrome/browser/banners/app_banner_data_fetcher.cc
[modify] https://crrev.com/bf032db305e5391644d614cc151a6599f8b0fbf6/chrome/browser/banners/app_banner_debug_log.cc
[modify] https://crrev.com/bf032db305e5391644d614cc151a6599f8b0fbf6/chrome/browser/banners/app_banner_debug_log.h
[modify] https://crrev.com/bf032db305e5391644d614cc151a6599f8b0fbf6/chrome/browser/manifest/manifest_icon_selector.cc
[modify] https://crrev.com/bf032db305e5391644d614cc151a6599f8b0fbf6/chrome/browser/manifest/manifest_icon_selector.h

If everyone's okay with the new messaging (Owen, it should be on Canary now to check), then I'll close this as Fixed.
I'm a little nervous about us showing different errors on different devices, but this is still a significant improvement from what we have before, so LGTM!

Thanks for doing this Dominick, it's really going to help developers get their stuff working without frustration!
Status: Fixed (was: Assigned)
Cc: dhadd...@chromium.org
Not sure how to verify this bug. 

On ChromeOS, when I go to the website in #1, i see "Add this site to your shelf to use it any time".

On my rentina mac I get this error message:
Refused to get unsafe header "X-MP-CE-Backoff"Sc @ mixpanel-2-latest.min.js:44

Is this expected functionality?
From #12, I assumed it would complain about the icon size.
dhaddock: do you have chrome://flags/#enable-add-to-shelf and chrome://flags/#bypass-app-banner-engagement-checks enabled? Banners will only show on Mac with those two flags on (they're enabled by default on ChromeOS, but not other desktop platforms).
I enabled both of those flags on my mac and still see the same error 

Chrome: 53.0.2785.143

Sign in to add a comment