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
Update support for installing non-PWA sites #34336
Update support for installing non-PWA sites #34336
Changes from 3 commits
3540c94
cbf4fd0
e4c855c
fd0af8c
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.
I'm not sure I like the reference to the Firefox extension. It's not official (as you say) or very widely used, and I'm not sure how referencing it really helps web developers. And it gives the docs a chance to be out of date or even dangerous if the extension stops getting support or turns out to have problems.
IMO we should just say it's not supported by Firefox.
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.
@wbamberg Yes ^^^.
Note, I haven't given up on reviewing this, just trying to clear my FF release stuff first. Feel free to update and merge it as you like, because I'm still trudging through the list.
What I want to do with this is the work to more loosely define what a PWA is and better capture app installability options. But those are bigger jobs.
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.
I also don't think we should list Safari as "supports installing PWAs". Safari support for installing sites is exactly what #30875 was designed to accommodate (see also #30463 (comment)), and I don't see any good reason to reconsider this here.
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.
Fair enough. I proposed a fix above.
The separate concepts we want to capture are manifest installation and app installation - a PWA can use either, but the manifest provides the best app-like experience.
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.
I think what's in the page now is entirely correct and reflects the "model" of the docs that the authors had in mind. I don't think any actual new information has emerged wrt Safari (i.e. we already knew that Safari supported installing any site) and what is/was known is already explicitly accommodated in the page. So I think we should leave it as is.
I don't know what the point is of adding more hedging about the manifest in here. But, I won't argue about it any more if you think it should be in. As i said already, I don't think arguing about exactly what defines a PWA is very productive.
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.
I don't feel strongly enough to make modifications the way I'd like without support. I'm OK to close the whole PR.
Though we should do this bit https://github.com/mdn/content/pull/34336/files#r1650338836 - since Chrome now supported for apps as well.