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 show "Adding new attribute failed" error message when loading of product screens is interrupted by page unload #38815

Merged
merged 4 commits into from Jun 22, 2023

Conversation

mattsherman
Copy link
Contributor

@mattsherman mattsherman commented Jun 20, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

Why did this happen

In WooCommerce 7.8 the empty state handling for the attributes tab on the product editor updated (#38126) and then refactored to add an alert when the background request to get the empty attribute form failed (#38354).

Unfortunately, in Firefox (and possibly other browsers), these background requests fail (and the error handling associated with them are executed) when they are canceled due to the entire page being unloaded (Chrome appears to handle this differently, not executing the error handling for these canceled/failed background requests).

To make matters worse, the request to get the empty attribute form was being made when on the product edit/list screen ("All Products": /wp-admin/edit.php?post_type=product), when it really only is useful on the product screen ("Add New": /wp-admin/post-new.php?post_type=product and "Edit": /wp-admin/post.php?post=0000&action=edit).

The page can get unloaded while these background requests are in-progress in normal circumstances when the connection between the browser and the server is slow.

How to reproduce the issue in #38755

A few scenarios where this can be seen:

  • Load "All Products" and quickly click a product to edit it, or sort the list by clicking one of the column headers
  • Load a product to edit it and quickly navigate to a different page

These can be difficult to reproduce if the timing isn't right. I found that setting my network throttling to "Wi-Fi" in the Network tab of the browser developer tools made it more likely that this occurred.

Also, as mentioned, I have only seen this happen in Firefox.

What is changed in this PR

This PR fixes this error by:

  • Only attempting to load the empty attribute form when editing a product (do not attempt to load it when viewing "All Products")
  • Suppressing the "Adding new attribute failed" alert if the page is being unloaded

This PR does not stop the meta-boxes-product.js and meta-boxes-product-variation.js files from being enqueued for the edit-product screen ("All Products"). While I believe it to be unnecessary in that scenario, those files have been loaded for 5-10 years in that scenario and I want to dig into things a bit further to verify that changing that will introduce no issues that aren't immediately apparent. I will create a follow-up issue to handle this.

Also, note that in Firefox there still may be circumstances where messages such as the following still appear in the console:

Uncaught (in promise) Object { code: "fetch_error", message: "You are probably offline." }

These messages appeared prior to WooCommerce 7.8. Ideally they would suppressed, but that would be a more involved fix and should be handled separately in the future if desired.

Closes #38755.

How to test the changes in this Pull Request:

With Firefox:

  1. Load "All Products" and quickly click a product to edit it, or sort the list by clicking one of the column headers
  2. Do this a number of times and verify that you never see the "Adding new attribute failed" alert
  3. Load a product to edit it and quickly navigate to a different page
  4. Do this a number of times and verify that you never see the "Adding new attribute failed" alert
  5. Repeat everything in Chrome to verify no issues exist there either
  6. Verify that the empty state for the attributes tab is correct as detailed in Update empty state for product attributes tab #38126.

Empty state for attributes tab:

Screenshot 2023-05-05 at 15 08 08

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jun 20, 2023
@mattsherman mattsherman changed the title Fix/adding new attribute failed error Do not show "Adding new attribute failed" error message when loading of product screens is interrupted by page unload Jun 20, 2023
@mattsherman mattsherman self-assigned this Jun 20, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 20, 2023

Test Results Summary

Commit SHA: 03ddbbe

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 50s
E2E Tests1890019020821m 41s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@mattsherman mattsherman force-pushed the fix/adding-new-attribute-failed-error branch from 72e04fc to 40b6187 Compare June 20, 2023 15:21
@mattsherman mattsherman mentioned this pull request Jun 20, 2023
5 tasks
@mattsherman mattsherman marked this pull request as ready for review June 20, 2023 20:41
@mattsherman mattsherman requested a review from a team June 20, 2023 20:41
@github-actions
Copy link
Contributor

Hi , @woocommerce/mothra

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@mattsherman mattsherman added type: bug The issue is a confirmed bug. focus: product Issues related to product or product page. focus: product management Related to product creation and editing. labels Jun 20, 2023
Copy link
Contributor

@nathanss nathanss left a comment

Choose a reason for hiding this comment

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

I was not able to reproduce the issue in Firefox with throttling, as described.

But based on your explanation and by reviewing the code, I think the fix makes a lot of sense, so I'm going to approve it.

@mattsherman mattsherman force-pushed the fix/adding-new-attribute-failed-error branch from d1e6c41 to 0b7cc80 Compare June 21, 2023 15:19
@mattsherman mattsherman force-pushed the fix/adding-new-attribute-failed-error branch from 0b7cc80 to 03ddbbe Compare June 21, 2023 20:24
@mattsherman mattsherman merged commit 6076458 into trunk Jun 22, 2023
17 checks passed
@mattsherman mattsherman deleted the fix/adding-new-attribute-failed-error branch June 22, 2023 01:26
@github-actions github-actions bot added this to the 8.0.0 milestone Jun 22, 2023
@jonathansadowski jonathansadowski modified the milestones: 8.0.0, 7.9.0 Jun 22, 2023
@jonathansadowski jonathansadowski restored the fix/adding-new-attribute-failed-error branch June 22, 2023 19:13
louwie17 pushed a commit that referenced this pull request Jun 22, 2023
…of product screens is interrupted by page unload (#38815)

* Suppress "adding new attribute failed" error message on page unload
* Only attempt to add empty attribute if page has attributes list
@jonathansadowski jonathansadowski modified the milestones: 7.9.0, 7.8.0 Jun 22, 2023
louwie17 pushed a commit that referenced this pull request Jun 22, 2023
…of product screens is interrupted by page unload (#38815)

* Suppress "adding new attribute failed" error message on page unload
* Only attempt to add empty attribute if page has attributes list
jonathansadowski pushed a commit that referenced this pull request Jun 22, 2023
Do not show "Adding new attribute failed" error message when loading of product screens is interrupted by page unload (#38815)

* Suppress "adding new attribute failed" error message on page unload
* Only attempt to add empty attribute if page has attributes list

Co-authored-by: Matt Sherman <matt.sherman@automattic.com>
jonathansadowski pushed a commit that referenced this pull request Jun 22, 2023
Do not show "Adding new attribute failed" error message when loading of product screens is interrupted by page unload (#38815)

* Suppress "adding new attribute failed" error message on page unload
* Only attempt to add empty attribute if page has attributes list

Co-authored-by: Matt Sherman <matt.sherman@automattic.com>
@oujisan2236
Copy link

thank you its been driving me nuts everytiime i click on anything under products on wp-admin

@ptepartz
Copy link

Still getting this error in 7.9.0 - using google chrome (fully updated).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: product management Related to product creation and editing. focus: product Issues related to product or product page. plugin: woocommerce Issues related to the WooCommerce Core plugin. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Adding new attribute failed."
5 participants