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

Jetpack Search: Ensure that the widget always persists an array #23476

Open
jsnmoon opened this issue Mar 16, 2022 · 9 comments
Open

Jetpack Search: Ensure that the widget always persists an array #23476

jsnmoon opened this issue Mar 16, 2022 · 9 comments
Assignees
Labels
Customer Report Issues or PRs that were reported via Happiness. Previously known as "Happiness Request". [Package] Search Contains core Search functionality for Jetpack and Search plugins [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Low [Type] Bug When a feature is broken and / or not performing as intended

Comments

@jsnmoon
Copy link
Member

jsnmoon commented Mar 16, 2022

Impacted plugin

Jetpack

Steps to Reproduce

Reported by one of our customers; exact repro steps are unknown.

What actually happened

It looks like a Jetpack Search widget was persisted into the options table as a non-array value, resulting in this warning notice being thrown:

PHP Warning: Undefined array key 'jetpack-search-filters-3' in /home/(redacted)/wp-includes/widgets.php on line 918

cc @StefMattana @kangzj

@jsnmoon jsnmoon added [Type] Bug When a feature is broken and / or not performing as intended [Pri] Low [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Package] Search Contains core Search functionality for Jetpack and Search plugins labels Mar 16, 2022
@StefMattana
Copy link

The error is reported in 4864304-zen. Not sure yet if it's useful to check for any plugin conflicts, I'm happy to follow up with the user about this if that's useful.

@matticbot matticbot added the Customer Report Issues or PRs that were reported via Happiness. Previously known as "Happiness Request". label Mar 17, 2022
@kangzj kangzj self-assigned this May 2, 2022
@kangzj
Copy link
Contributor

kangzj commented May 5, 2022

PHP Warning: Undefined array key 'jetpack-search-filters-3' in /home/(redacted)/wp-includes/widgets.php on line 918

The error suggests the option value stored in jetpack-search-filters doesn't have the particular key and also the value is an array. If the value is not an array, the error would be like:

Warning: Trying to access array offset on value of type bool in php shell code on line 1

I looked at all the places where we change the value of jetpack-search-filters, no place seems to add a widget to sidebars without adding in jetpack-search-filters option unless the logic stops in the middle of running.

Other possibilities could be, the option value is changed elsewhere.

update_option( $widget_opt_name, $widget_options );

update_option( $widget_opt_name, $widget_options );

I'll close the issue for now, unless more cases come in.

Thanks.

@kangzj kangzj closed this as completed May 5, 2022
@Ipstenu
Copy link

Ipstenu commented Dec 12, 2023

I've just run into this. Actually I have no idea how long it's been going on for but at the very least since December 4th (that's as far back as my logs go). I last updated Jetpack on December 5th, so it's not 'new'.

My guess is that it has to do with the fact that the 3rd and 4th options (categories and tags) are NOT present on all post types, and it's just flailing miserably when that happens.

Edit: Nope, still happening :(

2023/12/11 00:02:40 [error] 23649#23649: *8152699 FastCGI sent in stderr: "PHP message: 

PHP Warning:  Undefined array key "jetpack-search-filters-3" in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920PHP message: PHP Warning:  Trying to access array offset on value of type null in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920
PHP message: PHP Warning:  Undefined array key "jetpack-search-filters-4" in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920PHP message: PHP Warning:  Trying to access array offset on value of type null in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920
PHP message: PHP Warning:  Undefined array key "jetpack-search-filters-3" in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920PHP message: PHP Warning:  Trying to access array offset on value of type null in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920
PHP message: PHP Warning:  Undefined array key "jetpack-search-filters-4" in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920PHP message: PHP Warning:  Trying to access array offset on value of type null in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920
PHP message: PHP Warning:  Undefined array key "jetpack-search-filters-3" in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920PHP message: PHP Warning:  Trying to access array offset on value of type null in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920
PHP message: PHP Warning:  Undefined array key "jetpack-search-filters-4" in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920PHP message: PHP Warning:  Trying to access array offset on value of type null in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920
PHP message: PHP Warning:  Undefined array key "jetpack-search-filters-3" in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920PHP message: PHP Warning:  Trying to access array offset on value of type null in /

The only part of MY code that touches Jetpack search is this:

	/**
	 * Jetpack Search Filters
	 * We want to make sure we get post types in there.
	 */
	public function init_jetpack_search_filters() {
		if ( class_exists( 'Jetpack_Search' ) ) {
			// phpcs:disable
			\Jetpack_Search::instance()->set_filters( [
				'Content Type' => [
					'type'  => 'post_type',
					'count' => 10,
				],
				'Categories'   => [
					'type'     => 'taxonomy',
					'taxonomy' => 'category',
					'count'    => 10,
				],
				'Tags'         => [
					'type'     => 'taxonomy',
					'taxonomy' => 'post_tag',
					'count'    => 10,
				],
			] );
			// phpcs:enable
		}
	}

Source: https://jetpack.com/support/search/inline-search/customize-inline-search/#code-facets

However the error happens even with my code disabled.

Copy link
Contributor

Support References

This comment is automatically generated. Please do not edit it.

  • 4864304-zen
@pd-cm
Copy link

pd-cm commented Dec 12, 2023

Debugging the error is a bit challenging, as the reported error was 2 years ago and the version of WordPress is unknown. One could make some guesses based on changelog, but generally, this is the code section, with line numbers potentially shifted a bit, as there have been commits in widgets.php in WordPress core in the last two years.

https://github.com/WordPress/WordPress/blob/e99a11600602fbb5282b59227004d61c543d812d/wp-includes/widgets.php#L906-L929

function is_active_widget( $callback = false, $widget_id = false, $id_base = false, $skip_inactive = true ) {
        global $wp_registered_widgets;


        $sidebars_widgets = wp_get_sidebars_widgets();


        if ( is_array( $sidebars_widgets ) ) {
                foreach ( $sidebars_widgets as $sidebar => $widgets ) {
                        if ( $skip_inactive && ( 'wp_inactive_widgets' === $sidebar || str_starts_with( $sidebar, 'orphaned_widgets' ) ) ) {
                                continue;
                        }


                        if ( is_array( $widgets ) ) {
                                foreach ( $widgets as $widget ) {
                                        if ( ( $callback && isset( $wp_registered_widgets[ $widget ]['callback'] ) && $wp_registered_widgets[ $widget ]['callback'] === $callback ) || ( $id_base && _get_widget_id_base( $widget ) === $id_base ) ) {
                                                if ( ! $widget_id || $widget_id === $wp_registered_widgets[ $widget ]['id'] ) {
                                                        return $sidebar;
                                                }
                                        }
                                }
                        }
                }
        }
        return false;
}

In the original report, the error Undefined array key 'jetpack-search-filters-3' indicates an undefined array key is accessed, but line 918 is currently foreach ( $widgets as $widget ) {, making it an unlikely source for this warning. Mika's report of warning Trying to access array offset on value of type null from line 920 is similar... it could possibly be the same error from different PHP version's compilations, but taken literally, the first says an array key was accessed, but not found, while the second says a value within an undefined array key was accessed, but was nothing, not an array with an additional sub-key.

It is likely the file has since shifted down one or two lines, with the suspected code block being:

if ( ( $callback && isset( $wp_registered_widgets[ $widget ]['callback'] ) && $wp_registered_widgets[ $widget ]['callback'] === $callback ) || ( $id_base && _get_widget_id_base( $widget ) === $id_base ) ) {
	if ( ! $widget_id || $widget_id === $wp_registered_widgets[ $widget ]['id'] ) {

At the core, the problem is likely coming from the reality that this code segment is in a foreach which iterates each individual widget within each individual sidebar, but then attempts to get data about the widget from global $wp_registered_widgets.

The first of these lines checks that the desired value is set before accessing it:

if ( ( $callback && isset( $wp_registered_widgets[ $widget ]['callback'] ) && $wp_registered_widgets[ $widget ]['callback'] === $callback ) || ( $id_base && _get_widget_id_base( $widget ) === $id_base ) ) {

e.g., is there a global widget which has a callback set?

The second line does not check the same way -- it makes an assumption that if a callback is set, then so an id must also be set:

if ( ! $widget_id || $widget_id === $wp_registered_widgets[ $widget ]['id'] ) {

Therefore, it would likely resolve Mika's warning to verify id exists in $wp_registered_widgets[ $widget ] before accessing it.

The original warning may be the same thing, but generally, if being thorough, one might verify that $widget is an appropriately formatted array key which exists in $wp_registered_widgets and resolves to an array with both callback and id before manipulating the data or returning $sidebar, which, based on the function name, seems to be intended to indicate that the widget in question is active in a given sidebar.

This may be specific to Jetpack, but more generally seems to be a result of accessing data expected in a global based on a local list of widgets before verifying the global data takes the form expected from the last time the sidebar was saved.

One might expect this to occur in cases where the global registered widgets have changed in structure or do not have a widget ID defined. e.g., perhaps a widget was removed, a JetPack feature turned off when it previously was on, etc.

@Ipstenu
Copy link

Ipstenu commented Jan 2, 2024

Interestingly this has moved on:

2023/12/29 17:02:15 [error] 31060#31060: *11151161 FastCGI sent in stderr: "PHP message: PHP Warning:  Undefined array key "jetpack-search-filters-6" in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920; PHP message: PHP Warning:  Trying to access array offset on null in /home/wp_w9hpj2/lezwatchtv.com/wp-includes/widgets.php on line 920", client: IP-ADDRESS-REDACTED, server: lezwatchtv.com, request: "GET /post_type_shows-sitemap3.xml HTTP/1.1", host: "lezwatchtv.com"
@davisshaver
Copy link
Contributor

I'm seeing the same issue as @Ipstenu, actually with two Jetpack widgets (Top Posts and Search).

I did some digging and I believe this issue stems from is_active_widget() being called in the constructor for these module files. As noted in the Codex for is_active_widget(),

To be effective this function has to run after widgets have initialized, at action ‘init’ or later.

I've tested locally with moving the is_active_widget() into init actions. https://github.com/Automattic/jetpack/files/15139701/mypatch.patch

--- a/public/site/wp-content/plugins/jetpack/jetpack_vendor/automattic/jetpack-search/src/widgets/class-search-widget.php
+++ b/public/site/wp-content/plugins/jetpack/jetpack_vendor/automattic/jetpack-search/src/widgets/class-search-widget.php
@@ -70,13 +70,7 @@ class Search_Widget extends \WP_Widget {
 				'description' => __( 'Instant search and filtering to help visitors quickly find relevant answers and explore your site.', 'jetpack-search-pkg' ),
 			)
 		);
-
-		if (
-			Helper::is_active_widget( $this->id ) &&
-			! $this->is_search_active()
-		) {
-			$this->activate_search();
-		}
+		add_action( 'init', array( $this, 'init' ) );
 
 		if ( is_admin() ) {
 			add_action( 'sidebar_admin_setup', array( $this, 'widget_admin_setup' ) );
@@ -92,6 +86,15 @@ class Search_Widget extends \WP_Widget {
 		}
 	}
 
+	public function init() {
+		if (
+			Helper::is_active_widget( $this->id ) &&
+			! $this->is_search_active()
+		) {
+			$this->activate_search();
+		}
+	}
+
 	/**
 	 * Check whether search is currently active
 	 *
diff --git a/public/site/wp-content/plugins/jetpack/modules/widgets/top-posts.php b/public/site/wp-content/plugins/jetpack/modules/widgets/top-posts.php
index 8e792438b..e2b54cf62 100644
--- a/public/site/wp-content/plugins/jetpack/modules/widgets/top-posts.php
+++ b/public/site/wp-content/plugins/jetpack/modules/widgets/top-posts.php
@@ -72,10 +72,7 @@ class Jetpack_Top_Posts_Widget extends WP_Widget {
 
 		$this->default_title = __( 'Top Posts & Pages', 'jetpack' );
 
-		if ( is_active_widget( false, false, $this->id_base ) || is_customize_preview() ) {
-			add_action( 'wp_enqueue_scripts', array( $this, 'enqueue_style' ) );
-		}
-
+		add_action( 'init', array( $this, 'init' ) );
 		/**
 		 * Add explanation about how the statistics are calculated.
 		 *
@@ -87,6 +84,12 @@ class Jetpack_Top_Posts_Widget extends WP_Widget {
 		add_filter( 'widget_types_to_hide_from_legacy_widget_block', array( $this, 'hide_widget_in_block_editor' ) );
 	}
 
+	public function init() {
+		if ( is_active_widget( false, false, $this->id_base ) || is_customize_preview() ) {
+			add_action( 'wp_enqueue_scripts', array( $this, 'enqueue_style' ) );
+		}
+	}
+

Should we open a new issue about this?

@kangzj kangzj reopened this Apr 29, 2024
@kangzj
Copy link
Contributor

kangzj commented Apr 29, 2024

Thanks for the feedback @davisshaver. I reopened the issue and it's now under our radar; however we are not sure about the timeline to fix it. Thanks for proposing the fix, which is very helpful.

@Vrishabhsk
Copy link

hi 👋
I've been encountering the same issue as reported here. Is there a fix available for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Customer Report Issues or PRs that were reported via Happiness. Previously known as "Happiness Request". [Package] Search Contains core Search functionality for Jetpack and Search plugins [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Low [Type] Bug When a feature is broken and / or not performing as intended
8 participants