widget-options icon indicating copy to clipboard operation
widget-options copied to clipboard

Account for widget callbacks being array objects

Open westonruter opened this issue 5 years ago • 14 comments

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.

westonruter avatar May 06 '20 14:05 westonruter

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/

fellyph avatar Jan 28 '22 12:01 fellyph

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

medgedecastro avatar Feb 07 '22 04:02 medgedecastro

Hi @medgedecastro,

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

Thanks, Fellyph

fellyph avatar Feb 09 '22 17:02 fellyph

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?

hollisterca avatar Feb 09 '22 19:02 hollisterca

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

fellyph avatar Feb 09 '22 21:02 fellyph

We have pushed the suggested fixes. Thank you!

medgedecastro avatar Feb 25 '22 06:02 medgedecastro

Very nice. I noticed. 3.7.12 looks good so far

hollisterca avatar Feb 25 '22 06:02 hollisterca

@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.

westonruter avatar Feb 26 '22 05:02 westonruter

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

hollisterca avatar Mar 01 '22 05:03 hollisterca

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 avatar Mar 01 '22 08:03 medgedecastro

@hollisterca @westonruter

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

medgedecastro avatar Mar 02 '22 05:03 medgedecastro

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

  • Invalid markup removed: 34
  • CSS Usage went from 15% to 90% on rechecking a few pages

hollisterca avatar Mar 02 '22 05:03 hollisterca

@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 avatar Mar 02 '22 16:03 westonruter

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

westonruter avatar Mar 02 '22 16:03 westonruter