Fix cordova adapter concurrent loadstart handling#273
Conversation
|
@machete-in-space since you originally reported this issue, could I ask you to test the fix provided here to ensure it works? |
jonkoops
left a comment
There was a problem hiding this comment.
This will likely work good enough, we could clean up the Cordova code more, but right now I will take a simple fix, and it wouldn't make sense considering we are planning to remove the functionality entirely.
Signed-off-by: Uli <[email protected]>
6b3190e to
721e3b2
Compare
721e3b2 to
38315a8
Compare
|
@jonkoops sorry, didn't have the signoff commit message - fixed that now |
|
@jonkoops changes work on my end, thanks! |
|
@machete-in-space Thanks for validating! Does this also fix #209 for you? |
|
@jonkoops No, unfortunately this issue persists. There is still the attempt to load the redirect URL in the in-app-browser. As a workaround I am closing the browser manually on loadstart of the redirect URL. It's not clean but it does the job for now. |
|
Ok, then I will merge this in, we'll continue discussion on the issue itself. Thanks a lot for the review, and @eudamniac thanks for the PR! |
Restoring early completion of processing redirect uri load.
#208 is about an additional authenticate request, which seems to fail fail the login request since these changes in #102.
The completed flag before was set without an await on processCallback, which probably let to the duplicate authenticate request to be ignored since the flag was set to true already when the loadstart event is processed the second time. This PR would kind of restore the old logic by setting the completed flag before awaiting the #processCallback method.
Introducing integration test for the cordova adapters does not make sense anymore since the decision for removal in a future version. However, for reference the tests can be looked up here.
Closes #208