Closed
Bug 1409191
Opened 8 years ago
Closed 8 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)
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.
|
||
Updated•8 years ago
|
Whiteboard: [pwa-front-end]
|
|
||
Comment 1•8 years ago
|
||
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)
|
Assignee | |
Comment 2•8 years ago
|
||
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 | |
Updated•8 years ago
|
Assignee: nobody → cnevinchen
|
|
||
Updated•8 years ago
|
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
|
||
Comment 4•8 years ago
|
||
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)
|
|
||
Updated•8 years ago
|
Whiteboard: [pwa-front-end] → [FNC][SPT58.3][MVP][pwa-front-end]
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 9•8 years ago
|
||
Hi snorp, Dale,
Could you please help me review this code? Thank you!
Flags: needinfo?(snorp)
Flags: needinfo?(dharvey)
|
|
Assignee | |
Comment 12•8 years ago
|
||
Thanks for the sample in bug 1409759. I think the patch here can solve the problem.
Flags: needinfo?(bfrancis)
|
||
Comment 13•8 years ago
|
||
| mozreview-review | ||
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+
| Comment hidden (mozreview-request) |
|
||
Comment 15•8 years ago
|
||
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/23b3ec9f9b2a
Prefetch manifest before install. r=snorp
|
||
Comment 16•8 years ago
|
||
| bugherder | ||
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
|
|
||
Comment 17•8 years ago
|
||
Sorry was on PTO when this came up, but awesome job :)
Flags: needinfo?(dharvey)
Flags: needinfo?(snorp)
|
||
Comment 18•8 years ago
|
||
Verified the changes on the latest Nightly (2017-10-25).
Device:
Huawei Nexus 6P (Android 8.0)
Status: RESOLVED → VERIFIED
|
||
Updated•8 years ago
|
Flags: needinfo?(mliang)
|
Reporter | |
Updated•8 years ago
|
Flags: needinfo?(abovens)
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•