Skip to content

Fix APPLICATION_CANCELLED by handling back button press #1725

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

Merged
merged 11 commits into from
May 25, 2022

Conversation

nickbopp
Copy link
Collaborator

@nickbopp nickbopp commented Apr 25, 2022

AuthorizationActivity no longer needs to directly handle back button presses, and it's all standard machinery. (Web)AuthorizationFragment handle the press, and lifetime is tied to that of its view.

This change also updates the required androidx appcompat version from 1.0.2 -> 1.1. I expect most end-user applications are actually using an even newer version of this package already, as even 1.1 was released in 2019.

@nickbopp
Copy link
Collaborator Author

I'm still working on getting a good MSAL and OneAuth test pass using these Common changes, but I wanted to give folks a chance to look at the code.

@codecov
Copy link

codecov bot commented Apr 25, 2022

Codecov Report

Merging #1725 (f2bc049) into dev (1bb8a37) will not change coverage.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##                dev    #1725   +/-   ##
=========================================
  Coverage     15.62%   15.62%           
  Complexity      321      321           
=========================================
  Files           164      164           
  Lines          7097     7097           
  Branches        712      712           
=========================================
  Hits           1109     1109           
  Misses         5815     5815           
  Partials        173      173           
Impacted Files Coverage Δ
...ternal/providers/oauth2/AuthorizationActivity.java 0.00% <ø> (ø)
...ternal/providers/oauth2/AuthorizationFragment.java 0.00% <0.00%> (ø)
...iders/oauth2/CurrentTaskAuthorizationActivity.java 0.00% <ø> (ø)
...providers/oauth2/WebViewAuthorizationFragment.java 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e3a6a6...f2bc049. Read the comment docs.

@iamgusain
Copy link
Contributor

nit: Can you also add a link to any documentations on what changed between appCompat version 1.1.0 and 1.0.2 corresponding to the change in this PR, for future reference.

Copy link
Contributor

@iamgusain iamgusain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added few minor comments

@nickbopp nickbopp requested a review from iamgusain April 27, 2022 21:24
@nickbopp
Copy link
Collaborator Author

nickbopp commented May 6, 2022

When this PR is ingested to MSAL/OneAuth, the addToBackStack calls in the two EmbeddedBrowser classes should be removed. These changes together solve APPLICATION_CANCELLED + fragment lifetime being too long.

@nickbopp nickbopp marked this pull request as ready for review May 6, 2022 02:06
@nickbopp
Copy link
Collaborator Author

Okay, I'm back from OOF so this is ready for merge now, when someone with permissions is available to do so.

@iamgusain iamgusain merged commit 4bf1dfd into dev May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants