google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.25k stars 292 forks source link

1.106.0 conflict with WPML #7389

Closed jamesozzie closed 1 year ago

jamesozzie commented 1 year ago

Bug Description

As reported in the support forums, some WPML users are unable to access their WP admin or encounter fatal errors after upgrading to Site Kit 1.106.0.

One workaround that seemed to work for one user was the following:

  1. Navigate to "WPML > Settings > Post Types” and then set the post type Products to “Translatable – only show translated items“.
  2. Save this update
  3. Update to Site Kit 1.106.0 once more

For impacted users who may be severely impacted with the above workaround not resolving this issue, you can install Site Kit 1.105.0 where this should no longer occur. If you're having trouble deactivating and deleting Site Kit 1.106.0 please open a support topic.

Error Details
=============
An error of type E_ERROR was caused in line 3937 of the file /srv/htdocs/wp-content/plugins/sitepress-multilingual-cms/sitepress.class.php. Error message: Uncaught TypeError: Illegal offset type in isset or empty in /srv/htdocs/wp-content/plugins/sitepress-multilingual-cms/sitepress.class.php:3937
Stack trace:
#0 /srv/htdocs/wp-content/plugins/sitepress-multilingual-cms/classes/translations/class-wpml-post-element.php(54): SitePress->is_display_as_translated_post_type(Object(WP_Error))
#1 /srv/htdocs/wp-content/plugins/sitepress-multilingual-cms/classes/url-handling/wpml-url-filters.class.php(445): WPML_Post_Element->is_display_as_translated()
#2 /srv/htdocs/wp-content/plugins/sitepress-multilingual-cms/classes/url-handling/wpml-url-filters.class.php(238): WPML_URL_Filters->is_display_as_translated_mode(Object(WPML_Post_Element))
#3 /wordpress/core/6.2.2/wp-includes/class-wp-hook.php(310): WPML_URL_Filters->permalink_filter('https://XXX...', Object(WP_Post))
#4 /wordpress/core/6.2.2/wp-includes/plugin.php(205): WP_Hook->apply_filters('https://XXX...', Array)
#5 /wordpress/core/6.2.2/wp-includes/link-template.php(371): apply_filters('post_type_link', 'https://XXX...', Object(WP_Post), false, false)
#6 /wordpress/core/6.2.2/wp-includes/link-template.php(201): get_post_permalink(Object(WP_Post), false, false)
#7 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1099): get_permalink(Object(WP_Post))
#8 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(728): Google\Site_Kit\Core\Assets\Assets->get_product_base_paths()
#9 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(357): Google\Site_Kit\Core\Assets\Assets->get_inline_base_data()
#10 [internal function]: Google\Site_Kit\Core\Assets\Assets->Google\Site_Kit\Core\Assets\{closure}('googlesitekit-b...')
#11 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Script_Data.php(51): call_user_func(Object(Closure), 'googlesitekit-b...')
#12 [internal function]: Google\Site_Kit\Core\Assets\Script_Data->Google\Site_Kit\Core\Assets\{closure}('googlesitekit-b...')
#13 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Asset.php(129): call_user_func(Object(Closure), 'googlesitekit-b...')
#14 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1017): Google\Site_Kit\Core\Assets\Asset->before_print()
#15 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1026): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks(Object(WP_Scripts), Array)
#16 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1026): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks(Object(WP_Scripts), Array)
#17 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1026): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks(Object(WP_Scripts), Array)
#18 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(156): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks(Object(WP_Scripts), Array)
#19 /wordpress/core/6.2.2/wp-includes/class-wp-hook.php(308): Google\Site_Kit\Core\Assets\Assets->Google\Site_Kit\Core\Assets\{closure}('')
#20 /wordpress/core/6.2.2/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters(NULL, Array)
#21 /wordpress/core/6.2.2/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#22 /wordpress/core/6.2.2/wp-admin/admin-header.php(146): do_action('admin_print_scr...')
#23 /wordpress/core/6.2.2/wp-admin/index.php(137): require_once('/wordpress/core...')
#24 {main}

  thrown

We performed some internal testing on this and we were able to initially encounter errors, which required some database changes. After doing so we didn't encounter any errors specifically between Site Kit and WPML although we do see some PHP warnings relating to WPML (only with PHP 8.2.8 and not older versions)

Console errors with error reporting (and display) enabled, with only WPML active and SK disabled ``` Deprecated: Creation of dynamic property Whip_RequirementsChecker::$configuration is deprecated in /var/www/vhosts/plastiskip.com/httpdocs/wp-content/plugins/sitepress-multilingual-cms/vendor/yoast/whip/src/Whip_RequirementsChecker.php on line 37 Deprecated: Creation of dynamic property Whip_RequirementsChecker::$messageManager is deprecated in /var/www/vhosts/plastiskip.com/httpdocs/wp-content/plugins/sitepress-multilingual-cms/vendor/yoast/whip/src/Whip_RequirementsChecker.php on line 38 ``` ![image](https://github.com/google/site-kit-wp/assets/41326532/f3cbbad4-da38-40a9-9d6d-7df90219fc07)
Notes from team discussion > Currently, we create a fake product and try to get a permalink for it. It works fine on sites where there is nothing else that tries to modify the permalink. > In the reported site, WPLM tries to create a localised version of the permalink for non existing product and fails. > What we can do on our side: > Remove the current approach of finding the base path for products > Implement the new approach that will query one random product from the database and use it to detect the product base path > Found path will be cached for X minutes (probably something between 1-5 mins) to avoid querying the database for every request. >
Second stack trace ``` Stack trace: #0 /opt/bitnami/wordpress/wp-content/plugins/sitepress-multilingual-cms/classes/translations/class-wpml-post-element.php(54): SitePress->is_display_as_translated_post_type() #1 /opt/bitnami/wordpress/wp-content/plugins/sitepress-multilingual-cms/classes/url-handling/wpml-url-filters.class.php(445): WPML_Post_Element->is_display_as_translated() #2 /opt/bitnami/wordpress/wp-content/plugins/sitepress-multilingual-cms/classes/url-handling/wpml-url-filters.class.php(238): WPML_URL_Filters->is_display_as_translated_mode() #3 /opt/bitnami/wordpress/wp-includes/class-wp-hook.php(310): WPML_URL_Filters->permalink_filter() #4 /opt/bitnami/wordpress/wp-includes/plugin.php(205): WP_Hook->apply_filters() #5 /opt/bitnami/wordpress/wp-includes/link-template.php(371): apply_filters() #6 /opt/bitnami/wordpress/wp-includes/link-template.php(201): get_post_permalink() #7 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1099): get_permalink() #8 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(728): Google\Site_Kit\Core\Assets\Assets->get_product_base_paths() #9 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(357): Google\Site_Kit\Core\Assets\Assets->get_inline_base_data() #10 [internal function]: Google\Site_Kit\Core\Assets\Assets->Google\Site_Kit\Core\Assets\{closure}() #11 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Script_Data.php(51): call_user_func() #12 [internal function]: Google\Site_Kit\Core\Assets\Script_Data->Google\Site_Kit\Core\Assets\{closure}() #13 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Asset.php(129): call_user_func() #14 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1017): Google\Site_Kit\Core\Assets\Asset->before_print() #15 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1026): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks() #16 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1026): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks() #17 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1026): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks() #18 /opt/bitnami/wordpress/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(156): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks() #19 /opt/bitnami/wordpress/wp-includes/class-wp-hook.php(308): Google\Site_Kit\Core\Assets\Assets->Google\Site_Kit\Core\Assets\{closure}() #20 /opt/bitnami/wordpress/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters() #21 /opt/bitnami/wordpress/wp-includes/plugin.php(517): WP_Hook->do_action() #22 /opt/bitnami/wordpress/wp-admin/admin-header.php(146): do_action() #23 /opt/bitnami/wordpress/wp-admin/index.php(137): require_once('...') #24 {main} thrown in /opt/bitnami/wordpress/wp-content/plugins/sitepress-multilingual-cms/sitepress.class.php on line 3937 ```

Steps to reproduce

  1. Install Site Kit 1.105.0
  2. Install, set up and configure WPML
  3. Update Site Kit to version 1.106.0

(note that during testing I couldn't consistently reproduce this on different sites)

Screenshots

Related support topics

Additional Context


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Changelog entry

mxbclang commented 1 year ago

@aaemnnosttv Here was @eugene-manuilov's suggestion on a fix via Slack:

What we can do on our side: Remove the current approach of finding the base path for products Implement the new approach that will query one random product from the database and use it to detect the product base path Found path will be cached for X minutes (probably something between 1-5 mins) to avoid querying the database for every request.

robertstaddon commented 1 year ago

Just FYI, the exact issue occurred on my web server with PHP 8.1.21

mxbclang commented 1 year ago

Thanks, @robertstaddon! As you can see, we're working to address this on our side, but WPML has also notified us that they're planning to release a fix in their own update next week. You can follow along with them here: https://wpml.org/errata/site-kit-by-google-uncaught-typeerror-illegal-offset-type-in-isset-or-empty-in/.

strategio commented 1 year ago

Hi, I am Pierre from WPML and indeed we've included one more check on our side to prevent this kind of error. This will go out with WPML 4.6.5 scheduled for next Monday.

That said, I strongly support the idea of a better (less random) approach to discover the product URL format. As suggested by @eugene-manuilov, trying to fetch an existing product from the DB would probably be a more robust solution.

aaemnnosttv commented 1 year ago

Thanks @strategio, that's great to hear 👍 We'll use this issue to fix the problem people are experiencing now and we'll iterate on our implementation to be more robust in another as this is part of a future feature that isn't available yet.

Our next release is scheduled for the following Monday the 14th.

eugene-manuilov commented 1 year ago

IB ✔️

wpdarren commented 1 year ago

QA Update: ✅

Verified:

robertstaddon commented 1 year ago

Nice work! Thank you all.