-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix for #1416: set the classloader at Intent creation time. #1470
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 for #1416: set the classloader at Intent creation time. #1470
Conversation
This is because the exception happens _before_ we get to read the FlowParameters from the extras/bundle.
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.
Do you know if it works?
I'm trying it right now. Sorry, I was away for a few days, I tried to get it in now when I had a few mins... |
I'm sorry, I've tried a few times but I can't seem to get the compilation to work with the local build:
|
Oh, did you forget to remove the real FUI dependency? 😁 |
Not at all (excerpt from my build.gradle):
|
Hmmm, and you're sure that's the only time you declare those dependencies? Maybe try a |
Yes, I'm 100% sure:
And yes, I did a |
Huh, well I guess we might as well give it another go. PS: I hope you at least get a shirt out of all this. 😂 |
Lol -- I'd be ecstatic with a working patch.... :) |
@dimipaun thanks! Rather than blind-merge, I compiled an m2repo for you (attached). You can do:
That will let you resolve the |
Thank you, I'll give it a shot now. |
Sooo,
Here's a bit from the logcat:
|
@dimipaun the login screen is empty? Like no sign in method buttons appear at all? I also can't figure out how you're reproducing this. I've been using a Pixel emulator for years (and a real Pixel) and I've never had issues with the Intent extras like this. |
Yes it's crazy. I've also tried on the original device where I've identified the problem (Samsung S9+, Android 8.0.0 API 26). Same results:
Here is the relevant part from Logcat:
|
@dimipaun thanks for the testing. Clearly we're still missing something, and this fix is not enough. I don't feel comfortable moving forward until we understand what's different about your project or environment that causes the issue, but to be honest I am not even sure where to start! |
@samtstern I agree that something is odd. I've been able to reproduce this empty screen in the emulator, the S9+, and some other devices (like a Nexus 6 iirc). Now, the ClassNotFoundException seems to be particular to certain devices like my Samsung S9+, and my latest fix seems obviously correct (compared to all the others floating around the internet for the same problem). This is because without it we don't event get to start the AuthMethodPickerActivity activity. Now this works, so I think it's safe to merge it. The second part is why is it still empty once we are in the AuthMethodPickerActivity, since now we seem to be starting it properly. |
@samtstern Here's some more info: I've modified
I can confirm that the buttons are correctly added:
The screen looks empty (expect for the TOS and PP at the bottom). I was also able to capture the layout via the Layout Inspector: |
@samtstern @SUPERCILEX ... and if I tweak a bit the layout:
So there seems to be an issue with the layout. I'm not a Android layout expert, any thoughts? |
@dimipaun ah ok now we're getting somewhere! If this PR fixes the crash and reduces it to a layout bug, then I think we should merge. Can you file a new issue with everything you know about the layout bug? |
@dimipaun can you open a PR to remove the other two changes? Would be nice to not have dead code hanging around. |
OK, so this is merged, thanks guys! Thank you @samtstern for reverting those patches. I would have kept 1f3b18e in as it does seem to fix some problems (judging by others reporting similar issues), but it may be that this patch addresses them too. So let's try without it and see where we land. |
@dimipaun yeah I think this fix alone will do everything that the |
This is because the exception happens before we get to
read the FlowParameters from the extras/bundle.
Hey there! So you want to contribute to FirebaseUI? Before you file this pull request, follow these steps:
./gradlew check
to ensure the Travis build passes.