-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Fix broken system social share in Webviews and Opera UA detection #11556
Conversation
Why not just use a try catch? I dislike using UA detection if possible. |
It can't be used here. The API pops out the system share UI if no exception is thrown. |
I thought it was the |
@jridgewell discussed here: https://bugs.chromium.org/p/chromium/issues/detail?id=765923 (comment #6 and #7). yes UPDATE: |
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.
LGTM
systemShareSupported_() { | ||
// Chrome exports navigator.share in WebView but does not implement it. | ||
// See https://bugs.chromium.org/p/chromium/issues/detail?id=765923 | ||
const isChromeWebview = this.viewer_.isWebviewEmbedded() && |
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.
Should we also at least add a special case for the Facebook UA string?
Fixes #11473
Chrome does not implement System Share in WebViews (See https://bugs.chromium.org/p/chromium/issues/detail?id=765923 ) but exposes
navigator.share
API (throws exception on call) so we now we have to use some UA detection to see if system share is supported or not.Chrome 60+ also started sending
OPR
(opera) as part of user agent string, this PR also fixes that.