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 (backport #24497) #24637

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Feb 7, 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)

This is an automatic backport of pull request #24497 done by [Mergify](https://mergify.com).

…ls with blank screen (#24497)

(cherry picked from commit 7949414)
@mergify mergify bot requested a review from a team as a code owner February 7, 2025 15:39
@mergify mergify bot requested review from lmarceau and removed request for a team February 7, 2025 15:39
@yoanarios yoanarios removed the request for review from lmarceau February 7, 2025 15:40
@yoanarios yoanarios merged commit 88bb948 into release/v136 Feb 7, 2025
11 of 12 checks passed
@yoanarios yoanarios deleted the mergify/bp/release/v136/pr-24497 branch February 7, 2025 16:33
@@ -104,12 +104,11 @@ struct CustomUserAgentConstant {
private static let customDesktopUA = UserAgentBuilder.defaultDesktopUserAgent().clone(extensions: "Version/\(AppInfo.appVersion) \(UserAgent.uaBitSafari)")

static let customMobileUAForDomain = [
"paypal.com": defaultMobileUA,
"yahoo.com": defaultMobileUA,
Copy link
Collaborator

Choose a reason for hiding this comment

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

OOC why was yahoo.com removed also?

Copy link
Contributor

Choose a reason for hiding this comment

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

For both cases, they were returning defaultMobileUA which is the default for mobile so there was no need to list them as a customMobileUAForDomain. I added unit test in my other PR to cover Paypal for Mobile and DesktopUA

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 Thanks @yoanarios

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.

3 participants