Fix Sentry JETPACK-ANDROID-1G9S and 1J82: avoid openUrl with empty url - #23147
Closed
nbradbury wants to merge 1 commit into
Closed
Fix Sentry JETPACK-ANDROID-1G9S and 1J82: avoid openUrl with empty url#23147nbradbury wants to merge 1 commit into
nbradbury wants to merge 1 commit into
Conversation
ReaderActivityLauncher.openUrl() guards null/empty urls by reporting to Sentry and returning, so these two issues are handled reports rather than crashes. The root cause is two call sites passing empty urls, which also left the user with a button that silently does nothing. CommentDetailFragment only checked getAuthorUrl() != null, so an empty author url still wired up a dead click listener. WPWebViewActivity's "open in external browser" used mWebView.getUrl(), which is null until the first page load finishes; it now falls back to the URL_TO_LOAD extra. Fixes JETPACK-ANDROID-1G9S Fixes JETPACK-ANDROID-1J82
Collaborator
Generated by 🚫 Danger |
Contributor
|
|
Contributor
|
|
Contributor
Author
|
Closing this because neither is a crash. No data loss, no dead-end state. |
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
3 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.
Description
Fixes two Sentry issues that share a root cause:
Neither is a crash.
ReaderActivityLauncher.openUrl()already guards null/empty urls by logging a Sentry report and returning (added in #22830 to stop the original crash), so what's left is two call sites passing empty urls — each of which also leaves the user tapping a button that silently does nothing.CommentDetailFragmentonly checkedgetAuthorUrl() != null, so an empty author url still wired up a dead click listener on the commenter's name/avatar; it now falls through to the existing non-clickable styling.WPWebViewActivity's "open in external browser" usedmWebView.getUrl(), which is null until the first page load completes; it now falls back to theURL_TO_LOADextra, so the button works before load instead of no-opping. TheopenUrlguard itself is left in place as a safety net for other callers.Testing steps
READERwarning in logcat).