-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Android: Fix WebView crash for links of unknown schemes #10903
Conversation
When tapping on a link in a WebView with an unknown scheme, the app would crash. For example, if you have the link "something://example/" but your device doesn't have anything to handle the "something" scheme, the app would crash when the user clicks on the link. This change handles the exception to prevent the app from crashing. Instead, the app navigates to a blank page.
By analyzing the blame information on this pull request, we identified @jacobp100 and @sathyapriya-31 to be potential reviewers. |
I think this should be good to go in. Just to note, there is work around the same area in #10772, but I don't think there's a dependency between these two. Just noting this in case I am wrong and there does need to be coordination between the two. |
intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); | ||
view.getContext().startActivity(intent); | ||
} catch (ActivityNotFoundException e) { | ||
FLog.w(ReactConstants.TAG, "activity not found to handle uri scheme for: " + url, e); |
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.
How about dispatch event to JS side?
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.
@leeight Thanks for the suggestion. Is there an existing event you had in mind that should be fired? If not, is there a real world need for supporting a new event and firing it here? If not, I would prefer to leave the PR as is and we can add a JS event in the future if a real world need arises.
Previously, tapping on a link of an unknown scheme would navigate to a blank page. Now, instead the tap is a no-op and doesn't navigate anywhere. I think this is a better behavior.
Awesome, thank you for the fix! @facebook-github-bot shipit |
@mkonicek has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: When tapping on a link in a WebView with an unknown scheme, the app would crash. For example, if you have the link "something://example/" but your device doesn't have anything to handle the "something" scheme, the app would crash when the user clicks on the link. This change handles the exception to prevent the app from crashing. Instead, the click is a no-op and the WebView doesn't navigate anywhere. **Test plan (required)** Verified the app no longer crashes when clicking on unknown schemes in a test app. Also, my team uses this change in our app. Adam Comella Microsoft Corp. Closes facebook#10903 Differential Revision: D4226371 Pulled By: mkonicek fbshipit-source-id: a6d3957806c6063e74fe055b0979cb9d1ce40e51
When tapping on a link in a WebView with an unknown scheme, the app would crash. For example, if you have the link "something://example/" but your device doesn't have anything to handle the "something" scheme, the app would crash when the user clicks on the link. This change handles the exception to prevent the app from crashing. Instead, the click is a no-op and the WebView doesn't navigate anywhere.
Test plan (required)
Verified the app no longer crashes when clicking on unknown schemes in a test app. Also, my team uses this change in our app.
Adam Comella
Microsoft Corp.