-
Notifications
You must be signed in to change notification settings - Fork 99
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
#94 fixed event bus related app crushing issue on Android 5.0 Lollipop #96
Conversation
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.
Thank you for your contribution @bluetoothfx and sorry for the delay in reviewing.
While testing this I saw the crash reported on #98 and also forem#9809. Apparently when autofill takes control (during login) it calls the observeNetwork()
method, which in turn calls EventBus.getDefault().register(this)
and crashes (see screenshot below).
I think this can easily be fixed by calling EventBus.getDefault().unregister(this)
right before EventBus.getDefault().register(this)
inside the observeNetwork()
method from CustomWebViewClient.kt
. Since this PR refactors the EventBus implementation can you please include this one line for the fix as well? Thanks again for the help.
.idea/jarRepositories.xml
Outdated
@@ -0,0 +1,25 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
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.
I don't believe this file needs to be added, it can even be included in .gitignore
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.
added on .gitignore.
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 I remove this .idea/codeStyles/Project.xml also? I have updated kotlin and it appeared.
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.
I can see that we already had that one in source control. I think for now it's best if we do a separate and thorough PR for working out the files to include/ignore
@fdoxyz should I check before unregister or just straight unregister ?
|
I think the check would work great 👍 |
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.
Tested this and works great, including the suggested bug fix. Thank you again for the contribution @bluetoothfx
What type of PR is this? (check all applicable)
Description
This PR is related to DEV android app. If the application open from android version 5.0 (Lollipop) it crashes immediately & failed to perform application normal activity & shows Error Message "Unfortunately DEV stop working".
What was the error:
I have observed that the project was failing to initiate Event Bus. To properly initiate Event Bus I have followed the guideline
to implement it. Which is available here.
I need to add annotation processor which was lacking in this project. Which is highly recommended by Greenrobot team.
Which is mentioned here.
So I have made an application class and put them centrally & found that application working as expected.
Related Tickets & Documents
Please check following ticket to get details.
#94
Screenshots/Recordings (if there are UI changes)
N/A
[optional] What gif best describes this PR or how it makes you feel?