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

Bugfix FXIOS-9699 [webcompat] Force mobile UA for PayPal #24497

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

janbrasna
Copy link
Contributor

@janbrasna janbrasna commented Feb 1, 2025

📜 Tickets

Jira tickets: FXIOS-8027 (FXIOS-11230), FXIOS-9699
Github issues: #17911 (#24431), #21314

💡 Description

This moves the UAstring override for PayPal.com from the mobile section (where it defaults to the default anyways) to the desktop section, which correlates with the default setting for iPads; where most of the issues occur.

(Also: removes the default for yahoo.com, that also sends just default…)

NB: This is not a real fix of the underlying issue #17911 (comment) (as PayPal explicitly states that it should not be used from within WebViews, so whenever they insist on opening a popup the bug would demonstrate again) — this is just signaling them a mobile device, to open the widget in the same frame/tab, thus avoiding the about:blank breakage.

(In other words, getting desktop mode behavior for PayPal checkout buttons on par with with mobile mode, where issues are not being reported, for consistency.)

📝 Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • Updated the PR name to follow our PR naming guidelines
  • Wrote unit tests and/or ensured the tests suite is passing
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

@janbrasna janbrasna requested a review from a team as a code owner February 1, 2025 22:59
@janbrasna janbrasna requested a review from OrlaM February 1, 2025 22:59
@janbrasna janbrasna changed the title Refactor FXIOS-8027 [webcompat] Force mobile UA for PayPal Bugfix FXIOS-9699 [webcompat] Force mobile UA for PayPal Feb 3, 2025
@lmarceau lmarceau requested a review from yoanarios February 3, 2025 14:52
Copy link
Contributor

@yoanarios yoanarios left a comment

Choose a reason for hiding this comment

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

Hello @janbrasna thank you for opening this PR. I'm investigating Paypal bugs and I've tried a similar solution adding "paypal.com" to customDesktopUAForDomain but while testing I found that works only the first time if we close the popup and press Paypal button again it returns to desktopUA behaviour opening a blank page.

I've tested your changes and I'm seeing the same behaviour I will attach a video, although this is an edge case (closing the PopUp and press the button again) I think it will be best to have a solution that works every time

Simulator.Screen.Recording.-.iPad.Pro.11-inch.M4.-.2025-02-03.at.10.38.20.mp4

@OrlaM OrlaM removed their request for review February 3, 2025 16:28
@janbrasna
Copy link
Contributor Author

👋 Thanks @yoanarios — I will have to introspect their popup code a bit more in that case, to understand what they are trying to do, and how comes the handler changes like this 👀 — that would seem to actually get the desktop UA string from JS navigator object of the original merchant site, not just their scripts requested, but haven't really debugged their code what changes based on the user agent, TBD…

(I would never consider this to be a real fix, just wanted to improve the situation a little bit for now… as the UAstring overrides in their current form made no difference, this at least tries… ;D)

@yoanarios
Copy link
Contributor

👋 Thanks @yoanarios — I will have to introspect their popup code a bit more in that case, to understand what they are trying to do, and how comes the handler changes like this 👀 — that would seem to actually get the desktop UA string from JS navigator object of the original merchant site, not just their scripts requested, but haven't really debugged their code what changes based on the user agent, TBD…

(I would never consider this to be a real fix, just wanted to improve the situation a little bit for now… as the UAstring overrides in their current form made no difference, this at least tries… ;D)

That makes sense! Since this improves the flow, we can merge it. Meanwhile, I will continue working on fully resolving the blank page issue when the user closes the popup. Thanks for pushing this forward! 🚀

@yoanarios yoanarios merged commit 7949414 into mozilla-mobile:main Feb 6, 2025
13 checks passed
@yoanarios
Copy link
Contributor

@Mergifyio backport release/v136

Copy link
Contributor

mergify bot commented Feb 7, 2025

backport release/v136

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Feb 7, 2025
…ls with blank screen (#24497)

(cherry picked from commit 7949414)
yoanarios pushed a commit that referenced this pull request Feb 7, 2025
…4497) (#24637)

Bugfix FXIOS-9699 [webcompat] On online stores, Paypal connection fails with blank screen (#24497)

(cherry picked from commit 7949414)

Co-authored-by: Jan Brasna <[email protected]>
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.

2 participants