Skip to content
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

Merged
merged 5 commits into from
Nov 19, 2024

Conversation

amandabaker
Copy link
Member

Updates the same-origin Web Install explainer to support a 0-param installation (instead of requiring a single id param) and adds the requirement that the app to-be-installed must have a manifest with an id field.

@amandabaker amandabaker added the Web Install API Declarative install for web apps from a web app. label Oct 16, 2024
Copy link
Contributor

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some things to consider. Given the additional changes you're making here to essentially require id to be in the manifest for any same-origin installed app, I question if a 2-param signature still makes sense.

WebInstall/explainer_same_domain.md Outdated Show resolved Hide resolved
WebInstall/explainer_same_domain.md Show resolved Hide resolved
WebInstall/explainer_same_domain.md Show resolved Hide resolved
WebInstall/explainer_same_domain.md Show resolved Hide resolved
WebInstall/explainer_same_domain.md Show resolved Hide resolved
1. `navigator.install(id[[, install_url], <params>])`: The method takes an id (and optional install url) and tries to install the current origin. If the content being installed has a manifest file, this `id` must match the value in the manifest file. If there is no manifest file present, this `id` will act as the app id for the installed content. This is relevant since if the installed content were to be given a manifest file and made into an application, there is a way to automatically update the app going forward.
The call can also receive an object with parameters that it can use to customize a same domain installation. These parameters alter how the app is installed and are defined in an object. More information about the parameters is found in the [Parameters](#parameters) subsection of this specification.
1. `navigator.install()`: The method takes no parameters and tries to install the current document. If the content to be installed does not link to a manifest with a valid id, then installation will fail.
2. `navigator.install(id, install_url, [<params>])`: The method takes an id and install url and tries to install the web content at `install_url`. If the content to be installed does not have a linked manifest that specifies an id, and the provided `id` parameter does not match the computed id, then installation will fail.
Copy link
Contributor

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 of navigator.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 an id param at all? Should it just be a "one-param" method that takes the install_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 the id? 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?

Copy link
Member

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.)

Copy link
Contributor

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:

  1. The install_url that is (optionally) defined in the manifest.
  2. The 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 the install_url used in the API matches the one that is optionally specified in the manifest. The install_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 an id is and isn't expected in the manifest.

WebInstall/explainer_same_domain.md Show resolved Hide resolved
Copy link
Member

@diekus diekus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Will use as base for spec text.

@ThatMartianCat
Copy link

ThatMartianCat commented Nov 18, 2024 via email

@HowardWolosky
Copy link
Contributor

I've moved ahead with approving this so that we can get the 0-param change merged in, but I'm still concerned about parts of the changes in here and believe we'll need a follow-up.

  1. When we first were designing the API, it was permitting non-manifested sites to be installed. With this change, not only are we limiting its usage to manifested sites, we are further limiting it to manifested sites that have an id specified in their manifest (which is currently a low percentage across the web). Adoption of the API may be challenging if a majority of the desired sites do not have an id in their manifest and the caller has no ability to add it.

  2. If we're moving forward with this requirement, then it's not clear to me why we need to have an id passed-in at all. @dmurph originally proposed the required param to ensure that all web apps installed through this API had an explicitly specified id, regardless of whether there was a manifest with one specified or not. He made great arguments for that change, and I fully supported it. But if we're moving to a world where there must be a manifest and the manifest must have an id (see above), then it's no longer clear to me why we need to bother passing-in the id as part of the installation request. Is it intended to be defensive against the target web app substantially changing sometime after the source site added the code to install it, so we want to prevent installing the "wrong" web app? That seems like a bit of a reach. What I'd like to see is an update to the Explainer that covers the reasoning for why the developer should be providing the id in the two-param signature. What's in it for them?

/cc: @dmurph

@amandabaker
Copy link
Member Author

@HowardWolosky Sorry for the confusion. When we chatted offline, I wrongly said that an id was required in the manifest for 2 param installs because I confused it with the 0 param requirements. I updated the 2 param description again to require either an id or an install_url in the target manifest.

@amandabaker amandabaker merged commit 79613e5 into MicrosoftEdge:main Nov 19, 2024
1 check passed
@amandabaker amandabaker deleted the require-id-in-manifest branch November 19, 2024 19:08
@dmurph
Copy link

dmurph commented Nov 19, 2024

My understanding is that the 2-param version, for installing cross-origin, has the id required as the developer has no control over the cross-origin property. So it would limit the ability to create install buttons. I could see an argument against that... but it seems reasonable. For the same-origin 0-param version, the developer has full control over their own origin, so they can update their manifest to have the recommended id field. So - the 'pro' is that they are not blocked on another developer not setting an id. I think #894 (comment) addresses this.

(in general, encouraging adoption of an 'id' field in manifests everywhere will really help prevent problems in the future massively).

@HowardWolosky
Copy link
Contributor

My understanding is that the 2-param version, for installing cross-origin, has the id required as the developer has no control over the cross-origin property. So it would limit the ability to create install buttons. I could see an argument against that... but it seems reasonable. For the same-origin 0-param version, the developer has full control over their own origin, so they can update their manifest to have the recommended id field. So - the 'pro' is that they are not blocked on another developer not setting an id. I think #894 (comment) addresses this.

(in general, encouraging adoption of an 'id' field in manifests everywhere will really help prevent problems in the future massively).

The 0-param change totally makes sense to me. A dev should have access to the manifest file to add the id if they're trying to install themselves.

I think I still have a general question/concern regarding what the end-user (and developer) benefit is for the id in the two-param signature. Note: I fully acknowledge that I'm re-opening a previously closed discussion. Either I'm coming at this with a new perspective, or I've completely forgotten the previously convincing argument for why I'm wrong here (and would love to be reminded).

Scenario: I'm foo.com/news and I want to install foo.com/games/fun.

  • foo.com/games/fun has a manifest. No id specified but start_url = https://foo.com/games/fun.
  • foo.com/news uses navigator.install('https://foo.com/games/fun', 'https://foo.com/games/fun') which works because the resolved manifest id matches the id param.
  • Some time in the future, foo.com/games/fun adds an id to its manifest but sets the value to v2. That makes the resolved manifest id = https://foo.com/v2. Now, foo.com/news's navigator.install() call is going to start failing unless they make a change to be navigator.install('v2', 'https://foo.com/games/fun'). The original API call would still launch the previously installed app for old users (I think?), but it won't install for new users and it won't launch the app for users that recently installed the newer version themslves.

In this scenario, where was any benefit gained?

  • Not foo.com/news: The foo.com/news developer is likely not monitoring for PWA manifest changes to foo.com/games/fun, and probably doesn't have telemetry to alert them that this call is starting to fail for their users.
  • Not the end-user: End-users are likely to be confused why clicking the button on foo.com/news does nothing. It's true that we've prevented some users from having two different apps installed for the exact same site, but the end-user doesn't know that. All they know is that they are clicking the install button and nothing's happening.
  • Not the UA: We've prevented the navigator.install() API from being the one to potentially install a second version of the exact same app (but with a different resolved manifest id), however we haven't prevented the user from going directly to https://foo.com/games/fun, installing the site and then getting into the exact same situation that we were trying to prevent with naviagator.install().

It's possible that I'm misunderstanding something here. Which would be great. In that case, what we ultimately need is some clarity in the Explainer regarding why the id is necessary for the two-param signature (regarding what/who is ultimately benefitting from it being there).

If my analysis is right though, then we should consider dropping the id param since it's not gaining us much.

@dmurph
Copy link

dmurph commented Nov 19, 2024

@HowardWolosky said:

Some time in the future, foo.com/games/fun adds an id to its manifest but sets the value to v2. That makes the resolved manifest id = https://foo.com/v2. Now, foo.com/news's navigator.install() call is going to start failing unless they make a change to be navigator.install('v2', 'https://foo.com/games/fun'). The original API call would still launch the previously installed app for old users (I think?), but it won't install for new users and it won't launch the app for users that recently installed the newer version themslves.

This seems like a 'bad scenario' - /games/fun is now going to segment their user-base, there will now be two apps in the world... do they want that? That seems like they should never want that, unless it's an existentially different app.

Not foo.com/news: The foo.com/news developer is likely not monitoring for PWA manifest changes to foo.com/games/fun, and probably doesn't have telemetry to alert them that this call is starting to fail for their users.

What if the developer just hosted a totally different app there? How do we want that to fail?

Not the end-user

For the end-user - do they want to click on a button that says "install Foo" and then see an install dialog for "install Bar"?

If my analysis is right though, then we should consider dropping the id param since it's not gaining us much.

I think this is the 'most' OK if we require an 'id' field to be set in the manifest. However, it still means that the caller can no longer guarantee the app that's being installed - what if they have an ad deal here for AppA, and they're trying to install that, but it's not AppB? It seems maybe good to have confirmation?

I think this is not OK if the cross-origin install doesn't require the manifest to specify the id. Then - we have all of the same problems from here where developer can accidentally create irreversable issues where they end up with N apps in the world, instead of just 1.

If you were making a cross-origin install button, what would you want to happen if the dev at that other site made a mistake and:

  • Didn't host a manifest at all (should fail in all of our designs).
  • Accidentally changed the id of the application.
  • Hosted a totally different application there.

amandabaker pushed a commit that referenced this pull request Nov 19, 2024
updating the description of the install parameter based on #894

also, removing the part about adding to manifest's install_source as it no longer applies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Web Install API Declarative install for web apps from a web app.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants