Closed Bug 1347154 Opened 7 years ago Closed 7 years ago

Cannot install twitter as a PWA


(Firefox for Android Graveyard :: General, enhancement)

Not set


(firefox55 verified)

Firefox 55
Tracking Status
firefox55 --- verified


(Reporter: daleharvey, Assigned: daleharvey)




(1 file)

      No description provided.
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


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)
Assignee: nobody → dale
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) when installing

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
Flags: needinfo?(mcaceres)
Blocks: 1285858
Actually my test was premature, if I set TYPE_IMAGE then it fetches the icon successfully, also clarified the variable name
Flags: needinfo?(mcaceres)
Attachment #8848044 - Flags: review?(mcaceres)
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;
  iconURL new aWindow.URL(src);
} catch(err){
   return Promise.reject(err);

Alternatively, change fetchIcon() itself to be an async function?
Attachment #8848044 - Flags: review?(mcaceres) → review+
Pushed by
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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Verified as fixed on Nightly 55 (2017-03-31).
-HTC Nexus 9 (Android 7.1.1)
-Huawei Honor 8 (Android 6.0)
-LG G4 (Android 5.1)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.