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

Change Android WebView from 1 to ≤37 for API N-T #10653

Merged
merged 6 commits into from
Jun 6, 2021

Conversation

queengooborg
Copy link
Collaborator

A general rule of thumb that I heard and have been going by when updating BCD for WebView Android is that anything added in Chrome 1 will be present in WebView Android 1. However, looking at the difference in WebKit versions (528 for Chrome 1 vs. 523.12 for WebView Android 1), it's simply not plausible that this statement is true -- and after getting my hands on an Android 1.0 emulator, I've confirmed my suspicions. A more accurate rule of thumb would be that "anything present in Safari 3 will be in WebView Android 1".

This PR changes various entries set to WebView Android 1.0 to our range of ≤37 instead after running the mdn-bcd-collector in Android 1.0 and comparing the results to our data. I plan to follow up and replace these ranged values with proper version numbers in the future.

@github-actions github-actions bot added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label May 27, 2021
@@ -44,7 +44,7 @@
"version_removed": "2.0"
},
"webview_android": {
"version_added": "1",
"version_added": "≤37",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Due to mdn/yari#3238 this is going to end up looking broken on MDN. This was in Safari 1, so I'm guessing it was enabled with the rest of SVG in some early version of Android WebView? Can you identify that version to avoid this looking broken?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately I don't have access to many old Android versions at the moment, only 1.0, 1.5, and 4.4, so I can't determine a specific version yet...

@@ -44,7 +44,7 @@
"version_removed": "2.0"
},
"webview_android": {
"version_added": "1",
"version_added": "≤37",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This and other cases with "version_removed": "37" will end up looking broken on MDN due to mdn/yari#3238. It's not so broken that I think it has to change, but if you can find the right version for this now, that would be better.

@queengooborg queengooborg self-assigned this Jun 1, 2021
@foolip
Copy link
Collaborator

foolip commented Jun 1, 2021

For the SVG stuff, can you run npm run traverse webview_android all 1 | grep -i svg to ensure nothing is left? #10564 just changed some data that I think will need to be updated too.

@foolip
Copy link
Collaborator

foolip commented Jun 2, 2021

I've sent #10710 and running that against this PR would yield these errors:

Problems in 5 files:
✖ api/SVGAnimateColorElement.json
  Versions – 1 error:
  → api.SVGAnimateColorElement - version_removed: "37" must be greater than version_added: "≤37"
✖ api/SVGGraphicsElement.json
  Versions – 1 error:
  → api.SVGGraphicsElement - version_removed: "4.4" must be greater than version_added: "≤37"
✖ api/SVGSVGElement.json
  Versions – 2 errors:
  → api.SVGSVGElement.contentScriptType - version_removed: "37" must be greater than version_added: "≤37"
  → api.SVGSVGElement.contentStyleType - version_removed: "37" must be greater than version_added: "≤37"
✖ api/SVGTRefElement.json
  Versions – 1 error:
  → api.SVGTRefElement - version_removed: "4.4.3" must be greater than version_added: "≤37"
✖ api/SVGUseElement.json
  Versions – 2 errors:
  → api.SVGUseElement.animatedInstanceRoot - version_removed: "37" must be greater than version_added: "≤37"
  → api.SVGUseElement.instanceRoot - version_removed: "37" must be greater than version_added: "≤37"

I would suggest leaving those as "1" with an issue pointing out which entries are still known to be wrong, along with instructions (npm run traverse webview_android all 1 | grep -i svg) for checking if any more incorrect data has been added before the issue is resolved.

@foolip
Copy link
Collaborator

foolip commented Jun 2, 2021

But also, some of these entries can probably be removed to sidestep the whole problem.

@foolip
Copy link
Collaborator

foolip commented Jun 2, 2021

I've sent #10714 for one piece that can be dropped.

@foolip
Copy link
Collaborator

foolip commented Jun 3, 2021

Remaining lint errors if applying #10710:

Problems in 4 files:
✖ api/SVGAnimateColorElement.json
  Versions – 1 error:
  → api.SVGAnimateColorElement - version_removed: "37" must be greater than version_added: "≤37"
✖ api/SVGGraphicsElement.json
  Versions – 1 error:
  → api.SVGGraphicsElement - version_removed: "4.4" must be greater than version_added: "≤37"
✖ api/SVGSVGElement.json
  Versions – 2 errors:
  → api.SVGSVGElement.contentScriptType - version_removed: "37" must be greater than version_added: "≤37"
  → api.SVGSVGElement.contentStyleType - version_removed: "37" must be greater than version_added: "≤37"
✖ api/SVGTRefElement.json
  Versions – 1 error:
  → api.SVGTRefElement - version_removed: "4.4.3" must be greater than version_added: "≤37"

Since these have all been removed, I'd suggest leaving added as "1" and filing an issue about getting the right initial version for SVG in WebView, which involves updating both these remaining 1s and a lot of the new ≤37s.

@queengooborg
Copy link
Collaborator Author

I've opened #10790 to discuss this, and will revert the linter-conflicting changes for the time being, until I can find a way to test those other WebView versions!

@foolip foolip merged commit 21a90d1 into mdn:main Jun 6, 2021
@queengooborg queengooborg deleted the api/webview-1-to-range-n-t branch June 6, 2021 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants