Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Require id in manifest and support 0 param installation #894
Require id in manifest and support 0 param installation #894
Changes from 1 commit
2722d25
80148b1
ad09dfe
61b18aa
8ec723d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, to be clear, the API is now enforcing same-origin users to have a
id
defined in the manifest file, regardless of whether they're using the 0-param or the 2-param version. I generally feel ok with that, given that a dev adding same-origin usage ofnavigator.install
on a site should also be able to update the manifest file.That being said, if we're requiring a
id
to be explicitly defined in the manifest now for same-origin, does it make sense to have anid
param at all? Should it just be a "one-param" method that takes theinstall_url
and optional install params? It's not clear to me what the value/scenario is. Would a dev at/foo/
that is trying to install the app on/bar/
care about theid
? Wouldn't they just care about installing "that app" that's at/bar/
and not "that app but only if its id hasn't changed since I last wrote this code?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not every web app will have an
install_url
? I think having the 0 param makes sense to just install content that already is defined as an "app" with an id, and the 2 params is for when the developer wants to have a fine control of how to install content? (ie, custom install_url, matching ids and additional parameters.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are actually two different
install_url
's in play here:install_url
that is (optionally) defined in the manifest.install_url
that is the (required?) second parameter in the 2-param signature.There's no requirement for the manifest to have an
install_url
. and even if it does, there's no requirement that theinstall_url
used in the API matches the one that is optionally specified in the manifest. Theinstall_url
in the API signature simply tells the UA where to go to find the manifest.This mostly comes down to whether we are allowing the installation of web apps that don't have a manifest that at least specifies an app id. If an app id is going to be required to be specified in the web manifest for the API to be able to install it, then I don't see a compelling argument for specifying the app id in the API signature at all (at least there isn't a well documented one in the explainer so far). If we are allowing the API to install web apps that don't have an
id
specified in the manifest, then I think we should clearly document in the explainer with a table the full set of scenarios where anid
is and isn't expected in the manifest.