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

Do not redirect users after navigating back to order page. #187

Merged
merged 4 commits into from
Mar 25, 2024

Conversation

peterwilsoncc
Copy link
Contributor

@peterwilsoncc peterwilsoncc commented Dec 21, 2023

All Submissions:

  • Does your code follow the WooCommerce Sniffs variant of WordPress coding standards?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Will this change require new documentation or changes to existing documentation?

Changes proposed in this Pull Request:

This uses the performance navigation API to determine whether a user has navigated back from the payfast payment page to the WooCommerce Pay for order page.

If a user has navigated back, it prevents the JavaScript redirect from initiating and allows the user to pay or cancel the order via the buttons placed at the bottom of the screen.

Risks

It's using the PerformanceNavigationTiming API to sniff whether a user has navigated to the page via the back or forward buttons (it's not possible to determine the exact one of the two). If browser manufacturers decide in the future that this sort of thing is an abuse of the API then the code may stop working and need to be removed.

I don't think this use case is an abuse of the API but I can foresee a situation in which the spammier type of advertising service may start abusing it and prevent the rest of the web from having nice things. Again.

Closes #3.

Steps to test the changes in this Pull Request:

This will need testing in multiple browser but the reported bug in #3 relates to iOS Safari so be sure to test in that.

  1. Set the store's currency to South African Rand.
  2. Create a simple product of R20.00
  3. Go through checkout process using Payfast
  4. Once you are redirected to the Payfast gateway page, press back in your browser
  5. Two things:
    • You should not be redirected to the gateway page a second time.
    • You should not see the redirection overlay blocking access to the page.

Changelog entry

Fix - Allow navigation back from PayFast gateway payment page.

@ankitguptaindia
Copy link
Member

Hi @peterwilsoncc thanks for the PR. I am trying to reproduce this issue with a stable version but do not see spinner freezing issue after pressing the browser's back button. I tested with iPhone 14 with Safari 17.1 on iOS 17.1.

Can you please check if I am missing anything?

RPReplay_Final1703245063.MP4

@peterwilsoncc
Copy link
Contributor Author

peterwilsoncc commented Jan 7, 2024

@ankitguptaindia No, you don't seem to be missing anything. This is what I see on trunk in Safari on both iOS and on macOS.

payfast

In other browsers I was seeing the bug as you were getting it (pressing back would redirect to the payment page again).

Edit: Are you able to test with only the payfast gateway enabled? I'm wondering if that makes a difference?

@ankitguptaindia
Copy link
Member

Hi @peterwilsoncc,

I re-checked this again with:

I have a couple of thoughts on why there might be issues:

  • The problem was reported about 5 years ago, and since then, updates in iOS browsers or changes to the WooCommerce Checkout page might have fixed it.
  • It could be happening only in the real payment(Non-Sandbox)situations and we are testing with Sandbox environment.

@peterwilsoncc peterwilsoncc force-pushed the fix/3-back-to-order-page-ios branch from 2339870 to 6da0673 Compare January 25, 2024 03:00
@peterwilsoncc
Copy link
Contributor Author

@ankitguptaindia

I only see what you get above if I have the developer tools open and disable cache enabled in the network tab. Did you take the recording with those open?

Without network tool/a disabled cache I am still seeing the bug on trunk:

RPReplay_Final1706151527.MP4

Model: iPhone 7
iOS: 15.8

@ankitguptaindia
Copy link
Member

I only see what you get above if I have the developer tools open and disable cache enabled in the network tab. Did you take the recording with those open?

@peterwilsoncc This is still working for me on iPhone 14, iOS 17.2.1, Sarafri/Chrome. I am checking as a normal end user and didn't open developer tool.

Let's catch up over the screen share on Monday? we will try to reproduce it together.

@vikrampm1
Copy link
Contributor

@peterwilsoncc @ankitguptaindia this has been stuck in QA for a while so checking to see if it is blocked or can go to next step?

@peterwilsoncc
Copy link
Contributor Author

@ankitguptaindia and I compared results on this last night. The difference on trunk is occurring depending on whether the customer is logged in or logged out. Possibly due to the no-caching headers WordPress uses for logged in users, see source code.

@ankitguptaindia
Copy link
Member

Hello @peterwilsoncc,

I have checked these fixes, and now, after clicking the browser back button, the user is redirected back to the checkout page. However, the user encounters an issue where they cannot place an order upon returning to the checkout page. The "Place Order" button appears unresponsive.

It works fine for Safari but having issue on Chrome and Firefox.

Chrome:

Chrome.mp4

Safari-

Recording.530.mp4

Firefox-

Recording.531.mp4

@peterwilsoncc
Copy link
Contributor Author

Thanks Ankit. I'm now seeing different behaviour in multiple browsers from when we tested on Tuesday, including not being able to click the back button once to return from the Payfast gateway. I'll need to spend a little further time investigating.

@peterwilsoncc
Copy link
Contributor Author

@ankitguptaindia I've pushed 1c373bb which modifies the nocache headers to include no-store, private which prevents browsers from caching when pushing back buttons.

In Safari I am able to get back to the order-pay page, in Firefox I am sent back to the checkout page as the gateway is messing with the history.

I suspect the issue with the place order button being disabled is coming from this block of code in the WooCommerce repo.

https://github.com/woocommerce/woocommerce/blob/e38ffc8427ec4cc401d90482939bae4cddb69d7c/plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/place-order-button/index.tsx#L36-L41

I suspect it's intentional to prevent people double ordering accidentally. I've had a quick look for filters the payfast gateway can hook in to but nothing is evident base on a quick glance. For payfast it's probably a good enhancement to override this if the order hasn't been paid for.

@ankitguptaindia
Copy link
Member

QA/Test Report-

Testing Environment -

  • WordPress: 6.4.3
  • Theme: Storefront Version: 4.5.4
  • WooCommerce - Version 8.6.1
  • PHP: 8.0.22
  • Web Server: Nginx
  • Browser: Chrome - Version 121
  • OS: macOS Sonoma 14.3
  • Git Branch: fix/3-back-to-order-page-ios

Test Results - The Reported issue #3 and QA findings have been fixed/addressed. Tested this flow with iOS and macOS:

  • Safari
  • Chrome
  • Firefox
  • iOS Safari
  • iOS Chrome.
Recording.575.mp4

@ankitguptaindia
Copy link
Member

Regression+Smoke Test Report-

Testing Environment -

  • WordPress: 6.4.1
  • Theme: Storefront Version: 4.5.4
  • WooCommerce - Version 8.7.0
  • PHP: 8.3.0
  • Web Server: Nginx
  • Browser: Chrome , Safari, and Firefox
  • OS: macOS Monterey

Tested with Archive File created via

php woorelease.phar build smoke-testing branch _URL_

  • Composer version - v2.3.5
  • npm version - v8.19.2
  • node version - v16.18.1

Please note that this plugin has been tested with the build created by the specified versions ☝🏼 of Composer, Node, and NPM.

Status- Working as expected. Ready to merge 🚀

@vikrampm1 vikrampm1 modified the milestones: Future Release, 1.6.2 Mar 25, 2024
@vikrampm1 vikrampm1 marked this pull request as ready for review March 25, 2024 09:34
@vikrampm1 vikrampm1 mentioned this pull request Mar 25, 2024
20 tasks
@dkotter dkotter merged commit 37d896a into trunk Mar 25, 2024
5 checks passed
@dkotter dkotter deleted the fix/3-back-to-order-page-ios branch March 25, 2024 13:58
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.

Clicking the back button after purchase gives everlasting spinner on iOS.
4 participants