We are failing to fetch the icon on twitter when we attempt to install (there is also a small bug around ths icon fetching fallback code) If I comment out //request.overrideContentPolicyType(Ci.nsIContentPolicy.TYPE_WEB_MANIFEST); From the icon fetching code it works, it seems like the contentPolicy is saying it only wants to accept certain type of resources (I think I originally assumed it mean this was a manifest originating request)

Hrm, request.overrideContentPolicyType(Ci.nsIContentPolicy.TYPE_IMAGE); doesnt seem to work here either, its worth noting it is a cross origin request, this doesnt seem to happen when fetching other PWA's icons (which are mostly same origin) https://ma-0.twimg.com/twitter-assets/responsive-web/web/ltr/icon-ios.a9cd885bccbcaf2f.png when installing https://mobile.twitter.com/home Marcos any idea what is causing this and what is the harm of removing that line? twitter is most definitely a pwa we need to work

Actually my test was premature, if I set TYPE_IMAGE then it fetches the icon successfully, also clarified the variable name

Comment on attachment 8848044 [details] [diff] [review] Set correct csp for icon fetching Review of attachment 8848044 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay! Couple of things to consider, but generally all good :) ::: dom/manifest/ManifestIcons.jsm @@ +68,5 @@ > }); > } > > function fetchIcon(aWindow, src) { > + const iconURL = new aWindow.URL(src); If the URL might be relative, maybe always add a base? ``` new aWindow.URL(src, base); // but not sure what to use as base (aWindow.location?) ``` Also, beware that "new aWindow.URL(src);" can throw, so you might want to: ``` let iconURL; try{ iconURL new aWindow.URL(src); } catch(err){ return Promise.reject(err); } ``` Alternatively, change fetchIcon() itself to be an async function?

Pushed by dharvey@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8c14b29f1788 Set correct csp for icon fetching. r=marcosc

Thats great > Also, beware that "new aWindow.URL(src);" can throw, so you might want to The |.catch| in the the caller will catch that whether fetchIcon is async or not right, no need for a try catch? either way since its returning a promise no harm and a little clearer to make it async Add the base location on the way in too and landed, thanks

Verified as fixed on Nightly 55 (2017-03-31). Devices: -HTC Nexus 9 (Android 7.1.1) -Huawei Honor 8 (Android 6.0) -LG G4 (Android 5.1)