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

Update data for BeforeInstallPrompt event #12856

Merged
merged 7 commits into from
Nov 30, 2021
Merged

Update data for BeforeInstallPrompt event #12856

merged 7 commits into from
Nov 30, 2021

Conversation

queengooborg
Copy link
Contributor

This PR adds real values for Chromium (Chrome, Opera, Samsung Internet, WebView Android) for the onbeforeinstallprompt member of the Window API, based upon results from the mdn-bcd-collector project (v3.3.0).

Tests Used: https://mdn-bcd-collector.appspot.com/tests/api/Window/onbeforeinstallprompt

Check out the collector's guide on how to review this PR.

@queengooborg
Add Chromium versions for api.Window.onbeforeinstallprompt
dffbaf0 
This PR adds real values for Chromium (Chrome, Opera, Samsung Internet, WebView Android) for the `onbeforeinstallprompt` member of the `Window` API, based upon results from the [mdn-bcd-collector](https://mdn-bcd-collector.appspot.com) project (v3.3.0).

Tests Used: https://mdn-bcd-collector.appspot.com/tests/api/Window/onbeforeinstallprompt

_Check out the [collector's guide on how to review this PR](https://github.com/foolip/mdn-bcd-collector#reviewing-bcd-changes)._
@github-actions github-actions bot added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Oct 14, 2021
foolip
foolip reviewed Oct 23, 2021
View reviewed changes
api/Window.json Show resolved Hide resolved
api/Window.json Show resolved Hide resolved
@foolip
Copy link
Contributor

foolip commented Oct 30, 2021

Lint is still failing, it seems.

@sideshowbarker
Copy link
Member

Lint is still failing, it seems.

ah sorry, my fault — I only just now realized that https://wicg.github.io/manifest-incubations/ is a raw respec spec. That means I have to add it to the list of such specs in my tooling. After I do that and re-run the build, it should work. It’ll take about 30 minutes to run. I’ll post an update here when it’s done.

@sideshowbarker
Copy link
Member

Well, I fixed the problem I mentioned, and so that’s all working now — but the problem is this:

spec_urls only use allow listed hosts by w3c/browser-specs (and our exception list):

AssertionError [ERR_ASSERTION]: Invalid specification host(s) found. Try a more current specification URL and/or
check if the specification URL is listed in w3c/browser-specs.

@queengooborg queengooborg added the not ready This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. label Nov 22, 2021
@queengooborg queengooborg removed the not ready This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. label Nov 28, 2021
@queengooborg
Copy link
Contributor Author

Because this PR affects real values, I don't want the spec URL to get in the way of us reaching our real value goals, so I went ahead and removed them. The other event in the spec, onappinstalled, doesn't seem to have a spec URL assigned either. Let's add them in a follow-up PR!

@queengooborg queengooborg mentioned this pull request Nov 28, 2021
Closed
foolip
foolip requested changes Nov 29, 2021
View reviewed changes
api/Window.json Outdated
"description": "<code>beforeinstallprompt</code> event",
"support": {
"chrome": {
"version_added": "45"
Copy link
Contributor

Choose a reason for hiding this comment

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

Given #7844 I especially distrust Chrome 45 values so I dug deeper.

The feature was enabled by https://chromium.googlesource.com/chromium/src/+/e9e94d1355522e4e4a5a8aa2f540d265130ab98c which via https://www.chromium.org/developers/calendar maps to Chrome 44, although it's so close to the branch date it could have been 45.

But I've also tested logging BeforeInstallPromptEvent in BrowserStack, and can see that it was introduced in Chrome 44.

Can you update the data for BeforeInstallPromptEvent to 44 and also update this to match?

@queengooborg queengooborg changed the title Add Chromium versions for api.Window.onbeforeinstallprompt Update data for BeforeInstallPrompt event Nov 30, 2021
@foolip foolip merged commit 0f79a89 into mdn:main Nov 30, 2021
@queengooborg queengooborg deleted the api/Window/onbeforeinstallprompt/chrome branch November 30, 2021 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sideshowbarker sideshowbarker sideshowbarker left review comments

@foolip foolip foolip 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.
3 participants
@queengooborg @foolip @sideshowbarker