WidgetOptions / widget-options

Additional Widget options for better widget control. Available on
https://widget-options.com/
GNU General Public License v3.0
35 stars 16 forks source link

Account for widget callbacks being array objects #93

Open westonruter opened 4 years ago

westonruter commented 4 years ago

Fixes #64.

See also support forum topic: https://wordpress.org/support/topic/php-recoverable-fatal-error-when-switching-amp-mode/

This implements a fix I proposed in another support topic: https://wordpress.org/support/topic/standard-template-mode-internal-error/#post-12547053

This fixes compatibility with the AMP plugin which wraps all the registered widget callbacks with an object that implements ArrayAccess.

fellyph commented 2 years ago

Hi Folks,

Any update about this pull request? The users still having problems with issue #64 https://wordpress.org/support/topic/php-recoverable-fatal-error-when-switching-amp-mode/

medgedecastro commented 2 years ago

Hello there,

The developers tried to replicate the issue in one of our demo site. Unfortunately, we did not encounter the same error or issues.

For us to troubleshoot further, may we request for a staging site of the site with the issue? It might be related to other settings, plugins, and theme of the site.

Thank you, Mej

fellyph commented 2 years ago

Hi @medgedecastro,

I will ask the user to share some information about his site, plugins and environment.

Thanks, Fellyph

hollisterca commented 2 years ago

Hi @medgedecastro, I am the one that has these errors in the log file. You find some info here What else do you need to know? WP 5.9 Theme: GT, Version 2.8.6 by Graphene Themes Solutions Widget Option: Version 3.7.11 AMP: Version 2.2.1 Settings: Template Mode, Standard Under AMP - Validated URLs - for example on New Vegan French Toast I get A PHP fatal error occurred while validating the URL. This may indicate either a bug in theme/plugin code or it may be due to an issue in the AMP plugin itself. The error details appear below. If you are stuck, please search the support forum for possible related topics, or otherwise start a new support topic including the error message, the URL to your site, and your active theme/plugins. Please include your Site Health Info.

Uncaught Error: Object of class AMP_Validation_Callback_Wrapper could not be converted to string in /home/.../wp-content/plugins/widget-options/includes/widgets/extras.php:135
Stack trace:
#0 /home/.../wp-content/plugins/amp/includes/validation/class-amp-validation-callback-wrapper.php(144): widgetopts_sidebars_widgets()
#1 /home/.../wp-includes/class-wp-hook.php(307): AMP_Validation_Callback_Wrapper->__invoke()
#2 /home/.../wp-includes/plugin.php(189): WP_Hook->apply_filters()
#3 /home/.../wp-includes/widgets.php(1039): apply_filters()
#4 /home/.../wp-includes/widgets.php(702): wp_get_sidebars_widgets()
#5 /home/.../wp-content/themes/graphene/header.php(26): dynamic_sidebar()
#6 /home/.../wp-includes/template.php(770): require_once('/home/...')
#7 /home/.../wp-includes/template.php(716): load_template()
#8 /home/.../wp-includes/general-template.php(48): locate_template()
#9 /home/.../wp-content/themes/graphene/single.php(9): get_header()
#10 /home/.../wp-includes/template-loader.php(106): include('/home/...')
#11 /home/.../wp-blog-header.php(19): require_once('/home/...')
#12 /home/.../index.php(17): require('/home/...')
#13 {main}
  thrown

Location: /home/.../wp-content/plugins/widget-options/includes/widgets/extras.php:135

Can you take a look at the 2 commits from Weston? What else would you like to know?

fellyph commented 2 years ago

Hi @medgedecastro

The error is happening during the page validation, to reproduce the error follow those steps:

  1. Use AMP plugin on Standard or Transitional mode
  2. Add widget from the plugin on a page where there is an AMP version available
  3. In the AMP admin bar menu on the frontend, select "Validate URL"

Screen Shot 2022-02-09 at 21 32 53

medgedecastro commented 2 years ago

We have pushed the suggested fixes. Thank you!

hollisterca commented 2 years ago

Very nice. I noticed. 3.7.12 looks good so far

westonruter commented 2 years ago

@hollisterca But you've said the fix doesn't work?

@medgedecastro In looking at the code deployed to WordPress.org, I don't see the changes applied: https://plugins.trac.wordpress.org/browser/widget-options/tags/3.7.13/includes/widgets/extras.php#L129

I still see:

$id_base = is_array( $opts['callback'] ) ? $opts['callback'][0]->id_base : $opts['callback'];

And not not the changes suggested in this PR.

hollisterca commented 2 years ago

Just some feedback: on my prev. comment. So far was only a few minutes into testing. V 3.7.12 did not upgrade automatically but that was just a versioning problem, or was it. in V3.7.13 the upgrade worked again but I have issues again, the same ones I original posted. See feedback above from Weston. Hope you @medgedecastro can take another look why the code suggestions did not make it into the release. Thanks

medgedecastro commented 2 years ago

Hi @hollisterca @westonruter

It seems that the changes we need were gone when @edbertguinto push some updates. We will work on it.

Thank you,

medgedecastro commented 2 years ago

@hollisterca @westonruter

We have pushed the suggested fixes. Can you please confirm it is reflecting on your end.

hollisterca commented 2 years ago

@medgedecastro and @westonruter now on Version 3.7.14 The fatal errors seem to be gone in AMP I get now other issues

westonruter commented 2 years ago

@hollisterca That probably means it is working as expected. Previously validation could not be completed due to the fatal error, and so the amount of CSS was abbreviated and not all markup was rendered for AMP to validate.

westonruter commented 2 years ago

If there is no longer a fatal error, then it seems the Widget Options issue is fixed.