Support Android WebView for browser support detection#9557
Merged
Conversation
changelog: Internal, Browser Support, Improve browser detection for browser support
aduth
commented
Nov 7, 2023
| and_chr: ->(browser, version) { browser.android? && !browser.platform.android_webview? && browser.chrome?(version) }, | ||
| and_uc: ->(browser, version) { browser.android? && browser.uc_browser?(version) }, | ||
| chrome: ->(browser, version) { browser.chrome?(version) }, | ||
| android: ->(browser, version) { browser.platform.android_webview? && browser.send(:detect_version?, browser.version, version) }, |
Contributor
Author
There was a problem hiding this comment.
I don't love the send here, but android_webview? doesn't accept parameters like the other browser matchers. Internally, detect_version? is what the browser gem uses for parameterized methods (example).
An alternative implementation could be to use the same underlying core logic, but this risks some drift in the logic, and is more verbose.
Contributor
Author
|
Specs caught a real issue, I'll need to add support for |
zachmargolis
approved these changes
Nov 7, 2023
zachmargolis
reviewed
Nov 7, 2023
zachmargolis
approved these changes
Nov 7, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
🛠 Summary of changes
Enhances
BrowserSupportto account for Android WebViews.Previously, WebView user agents were often parsed as "Chrome", and would satisfy constraints for Chrome browser support. However, actual support in these WebView browsers would differ from the native Chrome browser. Since we were not treating these as "unsupported", we were seeing many errors being reported to our frontend error logger from webviews.
Updating the browserslist database also has the effect of including an Android minimum version (119).
The expected result here is that this will silence errors we're seeing from older WebView browsers.
📜 Testing Plan
Verify new browser support includes Android target:
make browsers.jsonbrowsers.jsonRun BrowserSupport tests:
If you're feeling ambitious, you can use one of the user agents in the specs to override the user agent string and confirm that error logging will not be added to the layout for an unsupported browser.