Automattic / woocommerce-payments

Accept payments via credit card. Manage transactions within WordPress.
https://wordpress.org/plugins/woocommerce-payments/
Other
173 stars 69 forks source link

WooCommerce payment is not working with AMP Standard #2713

Closed chrism245 closed 8 months ago

chrism245 commented 3 years ago

Steps to reproduce:

  1. Setup a WordPress.com (Business) or another WordPress (self-hosted) site with WooCommerce Payments and enable AMP Standard mode.
  2. Add a product to the cart and go to checkout.
  3. You can't complete the purchase, as there is no way to add the card information.
Markup 2021-08-11 at 09 34 38 Markup 2021-08-11 at 09 14 05

You will see the following message as well: "Since your browser does not support JavaScript, or it is disabled, please ensure you click the Update Totals button before placing your order. You may be charged more than the amount stated above if you fail to do so."

Markup 2021-08-11 at 09 15 57 Markup 2021-08-11 at 09 16 59

Related ticket: 4213285-zen

naman03malhotra commented 2 years ago

This can be helpful: p1647851793363319-slack-C01BPL3ALGP

theabhig commented 1 year ago

Just noting merchant in 5677052-zen also affected by this

timur27 commented 8 months ago

This comment is supposed to describe the problem itself, areas where it occurs, and the things to consider in terms of achieving compatibility.

How AMP in Standard mode modifies the page

When AMP in the Standard mode is enabled, it removes every incompatible or invalid markup from the page, leading to on of the options:

  1. the full page loading (no invalid markups found & removed)
  2. part of the page loading (some markups are invalid)
  3. the page not loading (all markup, or its main blocks are invalid).

As we can see, the result depends on the number of incompatibilities AMP will find on the page.

Which screen this issue describes and other screens behavior

The problem described in this issue occurs only on the shortcode checkout. Some part of the page (payment elements among others) was marked as invalid and thus removed from the page. Although it seems at first glance that just payment elements are removed, AMP marks some WooCommerce Core elements incompatible as well.

Now in comparison to shortcode checkout, Blocks checkout page doesn't load at all. This is because some WooCommerce Blocks & WooCommerce Core markup is invalidated and removed. It's worth mentioning, that Blocks checkout is the default checkout page now and it seems more important to work on compatibility issues there at first.

Having two checkout screens discussed now, we can infer that even assuming we provide AMP compatibility from WooPayments perspective, we still

  1. Have no guarantee that shortcode checkout will be fully compatible, because some checkout elements from WooCommerce Core are incompatible at the moment.
  2. Can say for sure that Blocks checkout will not work anyways because it will require providing compatibility from Blocks and from WooCommerce Core.

The above was the first sign for me which makes me hesitant about the need to fix this issue, at least starting from Payments.

How to make things compatible

There are couple of options to make things work, according to my short research.

  1. Switch to the Reader mode in AMP. With this, the pages with incompatibilities won't remove the invalid markdown but instead turn off the AMP. For us, it would mean that Blocks & shortcode will run correctly but not in the AMP performance mode for mobile.
  2. Keep the Standard mode on but manually mark all invalid markups on the checkout pages as Keep under AMP -> Settings. This way, we will disable AMP on checkout pages while still keeping AMP ON on other pages.
  3. Fix invalid markups to make them pass with AMP
    • e.g. we'll need to re-write tags like e.g. <img> to <amp-img>. Basically update our HTML to follow AMP components
    • we'll need to keep both implementations so that we support standard tags/elements so that when site doesn't use AMP we just use the default implementation
    • the scope of those changes will be big, because we'll need to "migrate" all the incompatible scripts to AMP
    • once we complete on the WooPayments side, it still won't be working e.g. in Blocks because of WC Core & Blocks incompatibilities I mentioned earlier

All this together with the conclusion that AMP is used in a very few sites (I'm assuming this based on the number of reports) makes me think if 1‍⃣ or 2‍⃣ is just enough to ask merchants to do if the problem occurs? Implementing 3‍⃣ seems like a really big work with very small impact.

I'd appreciate if we can analyse these details further and take a decision based on this research. cc @FangedParakeet I would really appreciate your opinion here 🙏 Thanks in advance!

FangedParakeet commented 8 months ago

Cheers for the investigation, @timur27!

Fix invalid markups to make them pass with AMP

I think updating all of our "invalid" tags to the AMP format will be challenging, but removing all external stylesheets and scripts will be a much greater obstacle that opposes the way that Woo/WordPress normally operates by enqueuing stylesheets and scripts onto the page.

I think this explains why the blocks checkout is almost completely non-functional in AMP standard mode, since almost the entire page is loaded dynamically, which defies the AMP philosophy. I consequently think removing our reliance on enqueuing will require a much larger refactor in order to bring us into the graces of the AMP environment.

I think for now I'm more comfortable declaring incompatibility with AMP standard mode or at least encouraging users to explore either of the available alternatives suggested above.

  1. Switch to the Reader mode in AMP. With this, the pages with incompatibilities won't remove the invalid markdown but instead turn off the AMP. For us, it would mean that Blocks & shortcode will run correctly but not in the AMP performance mode for mobile.
  2. Keep the Standard mode on but manually mark all invalid markups on the checkout pages as Keep under AMP -> Settings. This way, we will disable AMP on checkout pages while still keeping AMP ON on other pages.

@chrism245, it's been quite a while since you opened this issue, so apologies for the lengthy delay, but let me know, if possible, if either of these workarounds sounds acceptable.

@timur27, do you know how many tags/markups we would need to include in a "Keep" list in order to maintain checkout functionality?

For now, I am changing this issue from a bug to an enhancement and will return this to our enhancement triage queue for work to be prioritised for some later time.

timur27 commented 8 months ago

@timur27, do you know how many tags/markups we would need to include in a "Keep" list in order to maintain checkout functionality?

@FangedParakeet 28 tags should be kept on the shortcode checkout for checkout fields to be displayed correctly. Everything except Invalid attribute: as.

Since there are no action items at the moment for this issue, I'm unassigning myself for now. Please let me know if anything else will be needed and I'm happy to share what I've learned so far 👍

FangedParakeet commented 8 months ago

Since there are no action items at the moment for this issue, I'm unassigning myself for now. Please let me know if anything else will be needed and I'm happy to share what I've learned so far 👍

Agreed, thank you for your service. ✌️

I don't think we will be able to implement a solution that will allow complete compatibility AMP standard functionality and moreover there have been some workarounds suggested above. I'm closing this issue for now, since we will not currently be able to work on it, but please reopen if you disagree with this assessment, @chrism245. 🙏

chrism245 commented 8 months ago

Thanks for that @FangedParakeet!