-
Notifications
You must be signed in to change notification settings - Fork 21
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
Log Exception if an asset is enqueued before being registered #2576
Log Exception if an asset is enqueued before being registered #2576
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2576 +/- ##
===========================================
+ Coverage 64.5% 65.2% +0.6%
- Complexity 4591 4592 +1
===========================================
Files 796 475 -321
Lines 22992 17909 -5083
Branches 1234 0 -1234
===========================================
- Hits 14837 11671 -3166
+ Misses 7982 6238 -1744
+ Partials 173 0 -173
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @martynmjones, thanks for working on this! However, I wasn't able to reproduce the issue. I followed the steps but didn't get the fatal error. On my local machine I see the following when I go to Customize:
- The
plugin_loaded
hook is called, which registers the GlobalSiteTag Assets:
$this->register_assets(); |
- The
template_redirect
hook is called, andwp_footer
is triggered in the PIP plugin at:wp-content/plugins/woocommerce-pip/src/admin/class-wc-pip-customizer.php:880.
The PIP version that I am using is: 3.13.6
What am I doing differently?
Apologies @jorgemd24, for the error to be triggered in the example I gave, we need GLA to enqueue I've updated the test instructions to include installing and activating WP Consent API and making sure Google Analytics for WooCommerce is not active. You should then see the fatal error in the customizer. Apologies again for the trouble! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation! I was able to reproduce the issue, and I see that this PR catch the fatal error, so it looks good to me!
Changes proposed in this Pull Request:
Closes #1659
Adjusts asset enqueuing to catch InvalidAsset exceptions thrown when an asset is enqueued before being registered.
Detailed test instructions:
WooCommerce > Settings > Invoices/Packing Lists
Customize
underTemplate Customizer
InvalidAsset
exceptionfix/1659-asset-enqueued-before-registered
WooCommerce > Status > Logs
google-listings-and-ads-...
log fileChangelog entry