Closed Bug 1409191 Opened 5 years ago Closed 5 years ago

The "Add to home screen" prompt doesn't appear to use manifest values

Categories

(Firefox for Android Graveyard :: Web Apps (PWAs), enhancement, P1)

58 Branch
enhancement

Tracking

(firefox58 verified)

VERIFIED FIXED
Firefox 58
Tracking Status
firefox58 --- verified

People

(Reporter: abovens, Assigned: cnevinchen, NeedInfo)

References

Details

(Whiteboard: [FNC][SPT58.3][MVP][pwa-front-end])

Attachments

(1 file)

The "Add to home screen" prompt doesn't appear to use the values specified in the manifest.

We should use name (falling back to short_name) from the manifest instead of <title> from the <head>.

The URL should not be the current URL, but rather the URL deducted from the manifest's start_url value.
Whiteboard: [pwa-front-end]
Nevin, I ni-ed you just to let you be aware of this, let's triage the overall work for making the proper priority and MVP scope.
Flags: needinfo?(cnevinchen)
This means manifest parsing should happen before the user adds the launcher to home screen. This will need to mofidfy the current work flow instead of just adding badge and onboarding :) I also suggest we don't cache any information since this information may change at anytime. So extra network request will be needed for parsing the XML(1. Manifest content and 2.launcher icon)

My proposal is different with original author's concern(see bug 1393672 comment 7) so we may need more discussion before I start the work.
Flags: needinfo?(wehuang)
Flags: needinfo?(mliang)
Flags: needinfo?(jcheng)
Flags: needinfo?(dharvey)
Flags: needinfo?(cnevinchen)
Flags: needinfo?(alam)
Flags: needinfo?(abovens)
Assignee: nobody → cnevinchen
Flags: needinfo?(wehuang)
Priority: -- → P1
Gecko actually already downloads and parses the manifest before install via the Manifests JS module[0]. I don't like the way things work right now, though. We have WebAopManifest.java which handles the stuff we need in the standalone PWA activity, so I think we should just use that in Fennec before the install too. Gecko's involvement before install would be to simply deliver the manifest JSON. Nevin, I think you can probably make these changes yourself, but if you would like our help or need further clarification let me know.

https://dxr.mozilla.org/mozilla-central/rev/20d57b9c4183973af4af5e078dff2aec0b74f928/mobile/android/chrome/content/browser.js#2220
getManifest does not download the manifest, it just returns a stub value with installed=false, the manifest is downloaded on install. However I dont particularly like that it creates these stub manifests either anyway. 

On each page load a large number of things will be downloaded and the manifest may be a relatively small amount to download comparatively so whether you want to download the manifest when the install prompt is shown or on every page load is probably best decided between you and snorp / mobile owners etc. It is something I was concerned about but doesnt have to block features
Flags: needinfo?(dharvey)
Leaving this to Mark and Carol :)
Flags: needinfo?(alam) → needinfo?(chuang)
Whiteboard: [pwa-front-end] → [FNC][SPT58.3][MVP][pwa-front-end]
Hi snorp, Dale,
Could you please help me review this code? Thank you!
Flags: needinfo?(snorp)
Flags: needinfo?(dharvey)
I'll leave this to Mark :)
Flags: needinfo?(chuang)
Thanks for the sample in bug 1409759. I think the patch here can solve the problem.
Flags: needinfo?(bfrancis)
Comment on attachment 8919851 [details]
Bug 1409191 - Prefetch manifest before install.

https://reviewboard.mozilla.org/r/190772/#review196440

::: mobile/android/base/java/org/mozilla/gecko/webapps/WebAppManifest.java:71
(Diff revision 3)
>              Log.e(LOGTAG, "Failed to read webapp manifest", e);
>              return null;
>          }
>      }
>  
> +    public static WebAppManifest build(final String url, final String input) {

I like fromString() or similar as a name here, but not a big deal.
Attachment #8919851 - Flags: review?(snorp) → review+
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/23b3ec9f9b2a
Prefetch manifest before install. r=snorp
https://hg.mozilla.org/mozilla-central/rev/23b3ec9f9b2a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Sorry was on PTO when this came up, but awesome job :)
Flags: needinfo?(dharvey)
Verified the changes on the latest Nightly (2017-10-25).
Device: 
Huawei Nexus 6P (Android 8.0)
Status: RESOLVED → VERIFIED
Flags: needinfo?(mliang)
Flags: needinfo?(abovens)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.