Instabug 64/ Debug crash in WebView dropdown#718
Conversation
lealobanov
changed the title
PR SummaryFixed a crash in WebView dropdown by properly wrapping context with CrowdIn. Upgraded CrowdIn SDK to version 1.13.1 and improved its initialization. Removed unnecessary onPause override in DeepLinkingActivity. Changes
autogenerated by presubmit.ai |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Files Processed (4)
- app/src/main/java/com/flowfoundation/wallet/page/component/deeplinking/DeepLinkingActivity.kt (1 hunk)
- app/src/main/res/layout/activity_webview.xml (1 hunk)
- app/src/main/res/layout/webview_dropdown_item.xml (1 hunk)
- app/src/main/res/values/webview_styles.xml (1 hunk)
Actionable Comments (0)
Skipped Comments (2)
-
app/src/main/res/layout/webview_dropdown_item.xml [2-13]
enhancement: "Consider accessibility improvements for dropdown items"
-
app/src/main/res/values/webview_styles.xml [15-16]
best practice: "Avoid hardcoding text colors"
There was a problem hiding this comment.
🚨 Pull request needs attention.
Review Summary
Commits Considered (1)
- b339d93: Bump CrowdIn to latest
Files Processed (3)
- app/build.gradle (1 hunk)
- app/src/main/java/com/flowfoundation/wallet/FlowWalletApplication.kt (2 hunks)
- app/src/main/java/com/flowfoundation/wallet/page/component/deeplinking/DeepLinkingActivity.kt (1 hunk)
Actionable Comments (1)
-
app/src/main/java/com/flowfoundation/wallet/page/component/deeplinking/DeepLinkingActivity.kt [81-82]
possible bug: "Incomplete exception handling in URI validation"
Skipped Comments (1)
-
app/src/main/java/com/flowfoundation/wallet/FlowWalletApplication.kt [13-13]
best practice: "Add error handling for third-party SDK initialization"
| return try { | ||
| val uriString = uri.toString() |
There was a problem hiding this comment.
The isWalletConnectUri() method has a try block but the corresponding catch block is not shown in the diff. Ensure proper exception handling is implemented to avoid potential crashes when parsing invalid URIs.
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (1)
- 8851556: Bump CrowdIn to latest
Files Processed (1)
- app/build.gradle (1 hunk)
Actionable Comments (0)
Skipped Comments (1)
-
app/build.gradle [350-350]
best practice: "Consider specifying version constraints for dependency"
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (1)
- f0ef454: Bump CrowdIn to latest
Files Processed (1)
- app/src/main/java/com/flowfoundation/wallet/crowdin/CrowdinUtils.kt (1 hunk)
Actionable Comments (0)
Skipped Comments (1)
-
app/src/main/java/com/flowfoundation/wallet/crowdin/CrowdinUtils.kt [15-15]
maintainability: "Missing documentation for critical configuration change"
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (1)
- b3862f4: Bump CrowdIn to latest
Files Processed (1)
- app/src/main/java/com/flowfoundation/wallet/crowdin/CrowdinUtils.kt (1 hunk)
Actionable Comments (0)
Skipped Comments (1)
-
app/src/main/java/com/flowfoundation/wallet/crowdin/CrowdinUtils.kt [14-14]
maintainability: "Documentation needed for important configuration setting"
There was a problem hiding this comment.
🚨 Pull request needs attention.
Review Summary
Commits Considered (1)
- 24048f2: Bump CrowdIn to latest
Files Processed (2)
- app/src/main/java/com/flowfoundation/wallet/crowdin/CrowdinUtils.kt (1 hunk)
- app/src/main/java/com/flowfoundation/wallet/page/browser/widgets/LilicoWebView.kt (2 hunks)
Actionable Comments (1)
-
app/src/main/java/com/flowfoundation/wallet/page/browser/widgets/LilicoWebView.kt [67-71]
possible bug: "Missing initialization call in constructor"
Skipped Comments (1)
-
app/src/main/java/com/flowfoundation/wallet/page/browser/widgets/LilicoWebView.kt [59-65]
maintainability: "Repeated context wrapping logic in constructors"
| constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super( | ||
| context, | ||
| Crowdin.wrapContext(context), | ||
| attrs, | ||
| defStyleAttr | ||
| ) { |
There was a problem hiding this comment.
The third constructor is missing initWebView() call that is present in the other two constructors. This inconsistency could lead to incomplete initialization of the WebView when using this constructor.
3 participants
Related Issue
Closes #???
Summary of Changes
LilicoWebViewto properly handle framework resource lookupsNeed Regression Testing
Risk Assessment
Additional Notes
Screenshots (if applicable)