Skip to content
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 browser redirect on firefox browser. #4046

Merged
merged 2 commits into from
Jul 27, 2021

Conversation

michelleb-stripe
Copy link
Contributor

@michelleb-stripe michelleb-stripe commented Jul 25, 2021

Summary

Make the StripeBrowserLauncherActivity a single instance so that it clears the activities up until the StripeBrowserLauncher. After this change the redirect will now work on the firefox mobile browser.

I found this similar issue: https://stackoverflow.com/questions/55077642/android-deep-link-firefox-like-chrome

For us the concern about the extras is less relevant because in our code base onResult is called which just calls finish (it also then calls onNewIntent). It does not use the extras.

Attempt to fix #3952

Testing

  • Added tests
  • Modified tests
  • Manually verified

…lears the activities up until the StripeBrowserLauncher.

Attempt to fix 3952
@github-actions
Copy link
Contributor

github-actions bot commented Jul 25, 2021

Diffuse output:

OLD: paymentsheet-example-release-master.apk (signature: none)
NEW: paymentsheet-example-release-pr.apk (signature: none)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │  11.4 MiB │  11.4 MiB │  0 B │    39 MiB │    39 MiB │  0 B 
     arsc │   1.3 MiB │   1.3 MiB │  0 B │   1.3 MiB │   1.3 MiB │  0 B 
 manifest │   2.5 KiB │   2.5 KiB │ +1 B │  10.5 KiB │  10.5 KiB │  0 B 
      res │ 695.6 KiB │ 695.6 KiB │ -2 B │     1 MiB │     1 MiB │  0 B 
    asset │  23.9 KiB │  23.9 KiB │  0 B │  36.9 KiB │  36.9 KiB │  0 B 
    other │ 150.7 KiB │ 150.7 KiB │  0 B │ 308.1 KiB │ 308.1 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │  13.6 MiB │  13.6 MiB │ -1 B │  41.7 MiB │  41.7 MiB │  0 B 


         │          raw           │           unique            
         ├────────┬────────┬──────┼────────┬────────┬───────────
 DEX     │ old    │ new    │ diff │ old    │ new    │ diff      
─────────┼────────┼────────┼──────┼────────┼────────┼───────────
   files │      3 │      3 │    0 │        │        │           
 strings │ 177066 │ 177066 │    0 │ 164684 │ 164684 │ 0 (+0 -0) 
   types │  31122 │  31122 │    0 │  29644 │  29644 │ 0 (+0 -0) 
 classes │  27364 │  27364 │    0 │  27364 │  27364 │ 0 (+0 -0) 
 methods │ 157470 │ 157470 │    0 │ 153493 │ 153493 │ 0 (+0 -0) 
  fields │ 107721 │ 107721 │    0 │ 107431 │ 107431 │ 0 (+0 -0) 


 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  292 │  292 │  0   
 entries │ 4310 │ 4310 │  0
APK
   compressed   │  uncompressed   │                       
─────────┬──────┼──────────┬──────┤                       
 size    │ diff │ size     │ diff │ path                  
─────────┼──────┼──────────┼──────┼───────────────────────
   984 B │ -2 B │    882 B │  0 B │ ∆ res/-I.9.png        
 2.5 KiB │ +1 B │ 10.5 KiB │  0 B │ ∆ AndroidManifest.xml 
─────────┼──────┼──────────┼──────┼───────────────────────
 3.5 KiB │ -1 B │ 11.4 KiB │  0 B │ (total)
MANIFEST
@@ -97,3 +97,3 @@
         android:exported=true
-        android:launchMode=1
+        android:launchMode=2
         android:name=com.stripe.android.payments.StripeBrowserLauncherActivity

@mshafrir-stripe
Copy link
Collaborator

Were you able to verify the behavior with Custom Tabs and without custom tabs across different browsers?

From https://inthecheesefactory.com/blog/understand-android-activity-launchmode/en, "This mode is rarely used.". I wonder if there are any negative tradeoffs from using this mode?

@michelleb-stripe
Copy link
Contributor Author

I am testing this with Opera, Firefox, and Chrome browsers. With singleTop it only works with Chrome and Opera. With singleTask and singleInstance it works with Firefox, Chrome, and Opera.

@michelleb-stripe
Copy link
Contributor Author

Are there additional browsers you think would be worth testing with?

@michelleb-stripe
Copy link
Contributor Author

Just wanted to check on your intent of launchMode: singleTop (the original). Was your hope to destroy the activities above StripeBrowserLauncherActivity? In this case either singleTask or singleInstance would both work.

@mshafrir-stripe
Copy link
Collaborator

I am testing this with Opera, Firefox, and Chrome browsers. With singleTop it only works with Chrome and Opera. With singleTask and singleInstance it works with Firefox, Chrome, and Opera.

Is this also true for Custom Tabs vs not?

I think this change is fine, I was mainly wondering if there are any tradeoffs. Also sounds like singleTask is a smaller scope version of singleInstance. Should we use that instead?

@michelleb-stripe
Copy link
Contributor Author

yep! singleTask works for chrome tabs as well.

@@ -32,7 +32,7 @@
<activity
android:name=".payments.StripeBrowserLauncherActivity"
android:theme="@style/StripeTransparentTheme"
android:launchMode="singleTop"
android:launchMode="singleTask"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to singleTask

Copy link
Collaborator

@mshafrir-stripe mshafrir-stripe Jul 27, 2021

Choose a reason for hiding this comment

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

Doesn't have to be in this PR, but would be nice to add a comment as to why this is necessary.

@michelleb-stripe michelleb-stripe merged commit a000c14 into master Jul 27, 2021
@michelleb-stripe michelleb-stripe deleted the michelleb/fix-firefox-redirect branch July 27, 2021 10:27
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.

Mi Note - 7s Android app not redirecting back to app after completing a PaymentIntent Stripe
4 participants