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.

Sign up for GitHub

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

Jump to bottom

Chrome 71+ supports Permission API "persistent-storage" property #4876

Merged
merged 2 commits into from
Sep 27, 2019
Merged

Chrome 71+ supports Permission API "persistent-storage" property #4876

merged 2 commits into from
Sep 27, 2019

Conversation

bershanskiy
Copy link
Contributor

Summary

Chrome 71 added Permission API "persistent-storage" property on all platforms (desktop, Android, WebView).

Data

Chrome Platform Status:
https://chromestatus.com/feature/4770049554382848
Chromium bug:
https://crbug.com/878525

A checklist to help your pull request get merged faster:

  • Summarize your changes
  • Data: link to resources that verify support information (such as browser's docs, changelogs, source control, bug trackers, and tests)
  • Data: if you tested something, describe how you tested with details like browser and version
  • Review the results of the linter and fix problems reported (If you need help, please ask in a comment!)
  • Link to related issues or pull requests, if any

@ghost ghost added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Sep 24, 2019
queengooborg
queengooborg requested changes Sep 27, 2019
View reviewed changes
Copy link
Contributor

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

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

Hey there, thanks for your PR! A couple of things:

  1. I actually see no mention of WebView support on the ChromeStatus page you linked, which tells me that WebView doesn't have support at all. Can we set it to false?
  2. We can also fill in the Opera information. Opera should be "58" and Opera Android should be "50". Can we update them accordingly?
  3. Samsung Internet's latest release is based upon Chrome 67. Can we set it to false?

@bershanskiy
Copy link
Contributor Author

Thanks for your review!

We can also fill in the Opera information. Opera should be "58" and Opera Android should be "50". Can we update them accordingly?

Will do.

Samsung Internet's latest release is based upon Chrome 67. Can we set it to false?

Will do.

I actually see no mention of WebView support on the ChromeStatus page you linked, which tells me that WebView doesn't have support at all. Can we set it to false?

That's weird because:

  1. Intent to ship indicates "persistent-storage" is available on all platforms:

    Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?
    Yes.

  2. I tested it on Android WebView and it works. (The test case is attached below.)

Test

index.js - simple static server

var connect = require('connect')
var serveStatic = require('serve-static')
connect().use(serveStatic(__dirname)).listen(8080, function(){
    console.log('Server running on 8080...')
});

index.html - the test case

<body>
  <p>Permission status of persistent-storage is <a id="state"></a></p>
  <script>
    const text = document.getElementById("state")
    navigator.permissions.query({ name: "persistent-storage" })
      .then(permissionStatus => text.innerText = permissionStatus.state)
      .catch(error => text.innerText = "not available, due to error: " + error)
  </script>
</body>

Results: both in Chrome for Android and Android WebView the status is denied.

Review Feedback
ec5c827 
Add data for Opera (desktop and Android) and Samsung Internet (Android).
queengooborg
queengooborg approved these changes Sep 27, 2019
View reviewed changes
Copy link
Contributor

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

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

Well, if you’ve tested it in WebView and it functions as expected, then ChromeStatus is inaccurate. Alrighty, this looks good to me, thank you for the PR!

@bershanskiy
Copy link
Contributor Author

@vinyldarkscratch Can I do anything to advance this PR or are we just waiting for @jpmedley's review?

@queengooborg
Copy link
Contributor

Whoops, looks like my squash and merge didn't go through on my phone!

@queengooborg queengooborg merged commit 63319e1 into mdn:master Sep 27, 2019
@Sarrac3873 Sarrac3873 mentioned this pull request Oct 17, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@queengooborg queengooborg queengooborg approved these changes

Assignees
No one assigned
Labels
data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bershanskiy @queengooborg