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

Call _load_textdomain_just_in_time correctly #21742

Conversation

leonidasmi
Copy link
Contributor

@leonidasmi leonidasmi commented Oct 23, 2024

Context

Summary

This PR can be summarized in the following changelog entry:

  • Stops PHP notices about _load_textdomain_just_in_time loading incorrectly, on WP 6.7.

Relevant technical choices:

  • _load_textdomain_just_in_time is triggered by calling translating functions (like __() on hooks earlier than init. When we get() or set() Yoast options, we ultimately call WPSEO_Option::get_defaults() which in turn calls WPSEO_Option::translate_defaults() and WPSEO_Option::enrich_defaults() which contain such translating functions. Unfortunately, we have a couple of instances where we get() or set() Yoast options before the init hook, thus outputting those new notices
  • In order to not change the timing of those calls, as that would be super invasive, we went with enhancing the Options framework by allowing get()ing or set()ing to ask only for the options that are relevant and not all the Yoast options
  • Fortunately, most of the get()s and set()s that are called before init where about settings in the wpseo and wpseo_social options, which don't have those WPSEO_Option::translate_defaults() and WPSEO_Option::enrich_defaults() functions implemented. So, with the new enhancement of the Option framework, we are now able to ask for the defaults of only the wpseo and wpseo_social options before init
  • There were a few instances where settings from the wpseo_titles option were required and considering that wpseo_titles does have those WPSEO_Option::translate_defaults() and WPSEO_Option::enrich_defaults() functions implemented, we had to move those away from the pre-init hook, but we were able to do so seamlessly.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

(Note, all these steps and the impact check requires define( 'WP_DEBUG_LOG', true ); define( 'WP_DEBUG', true ); define( 'WP_DEBUG_DISPLAY', false ); in your config file)

  • Install WP 6.7 (at the time of the writing it's on beta3)
  • Change your site language to something other than english
  • Visit both the admin and the frontend of your site and confirm you dont get any PHP notices regarding Function load_plugin_textdomain was called incorrectly or Function _load_textdomain_just_in_time was called incorrectly
  • Also check your debug.log after the steps in Impact Check and confirm you still didnt get that notice

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

General smoke test of the additions in our Options framework:

  • Create two fresh sites
  • In Users->Profile, change the user's language to not English, eg. greek
  • Install the production versions of our Free in one and the new RC/PR in the other
  • Activate the plugin in both sites
  • Check in the database for the wpseo values in both sites.
  • Confirm that you get the same values for that option
    • One easy way to test is to run the serialized stored values in https://www.unserialize.com/ and compare the unserialized output.
    • You might get differences in options that save timestamps but that's expected. You will also get differences in the version option, but that's also expected and should indicate the version of the plugin you have installed
  • Now install the production versions of Premium, Local, Video and News plugins in one and the new RCs/PRs in the other (Local and News dont have a new PR/RC, just install their production version in both)
  • Check in the database for the wpseo_titles, wpseo_social, wpseo_taxonomy_meta, wpseo_premium, wpseo_video, wpseo_local and wpseo_news values in both sites
  • Confirm that you get the same values for all those options
    • For the index_now_key in wpseo, you will get a different value in each site, but make sure that it's populated with a string in both cases.
  • Go to the orphaned workout in both sites, complete the first step and recheck the wpseo_premium option. Confirm that the option still is the same in both sites.
  • With the Yoast Test Helper, activate books and movies
  • In Yoast SEO->Settings->Genres, confirm that the values are the same between sites
  • Also change some settings in both sites (ideally for all add-ons) and again compare that you get the same values in the database for all options.

Multisites:

  • Create two fresh multisites sites
  • In Users->Profile, change the user's language to not English, eg. greek
  • Install the production versions of our Free in one and the new RC/PR in the other
  • Activate the plugin in both sites
  • Check in the database for the wpseo_ms values in both sites (in the wp_sitemeta table).
  • Confirm that you get the same values for that option
  • Go to the network's Yoast SEO->General in both sites and disallow the same setting eg. SEO analysis
  • Again compare the value for the wpseo_ms option and again confirm it's the same in both sites.

Custom rewrite rules for categories:

  • Install Yoast SEO in a fresh install
  • In your wp-admin's wp-include/rewrite.php file, add a error_log( 'flush_rewrite_rules' ); line in the start of the flush_rewrite_rules() function
  • Go to Post->Categories, find a category and confirm that the URL contains the /category prefix (aka, http://example/category/cat3/
  • Go to that link and confirm you arrive at the expected page
  • Change the slug of that category to cat3-new, again visit that page and confirm you get to the expected page. Also confirm that your debug.log doesn't contain the flush_rewrite_rules log
  • Now to Yoast SEO->Settings->Categories and uncheck the Show the categories prefix in the slug option
  • Refresh the Post->Categories page and confirm that now the URL doesn't contain the /category prefix (aka, http://example/cat3-new/
  • Go to that link and confirm you arrive at the expected page
  • Change the slug of that category to cat3-new-new, again visit that page and confirm you get to the expected page. Also confirm that your debug.log contains the flush_rewrite_rules log.
  • Now go to your Settings->Permalink page and add something to the Category base setting, eg. cat. Then, clean your debug.log, again set Show the categories prefix in the slug to true and repeat all the above steps. (you now expect to see cat instead of category in the URLs of the categories, but only when Show the categories prefix in the slug is active.

Custom rewrite rules for categories (part 2):

  • In a fresh site, create a couple of categories before installing Yoast SEO
  • Install and activate Yoast SEO and go to the frontend of those categories
  • Confirm that the canonical meta tag for the categories have an appropriate value (and not eg. the URL of the homepage)
  • Go to Yoast SEO->Settings->Categories and disable the Show the categories prefix in the slug setting
  • Go to the frontend of the categories and again confirm that the canonical meta tag for the categories have an appropriate value (and not eg. the URL of the homepage)
  • Deactivate and reactivate the frontend of the categories and again confirm that the canonical meta tag for the categories have an appropriate value (and not eg. the URL of the homepage)

Get accessible post types:

  • Install Yoast SEO in a fresh install
  • In src\helpers\post-type-helper.php add error_log( print_r( $post_types, true ) ); under $post_types = \apply_filters( 'wpseo_accessible_post_types', $post_types );
  • Go to Yoast SEO->General page and confirm that you get the post types you expect in your debug.log, NOT including attachment
  • Undo your code modifications and once you save, go to Yoast SEO->Settings->Media page and enable the Enable media pages setting
  • Re-add the code, refresh the Yoast settings page and this time confirm that you now see attachment as well in the logged output.

Social settings metadata:

  • Go to create a post
  • Add Social title and X title and description
  • Save the post, reload and confirm that what you saved in your social settings are still there
  • Disable open graph or X from Yoast settings and repeat the test
  • Disable both and reload the edit post page. Confirm that you don't get the Social tab in Yoast metabox.

Update sitemap cache when post is published:

  • Make sure you have XML sitemaps enabled in your Yoast SEO settings
  • Create a draft post and then go to Posts->All posts
  • Visit the /post-sitemap.xml sitemap
  • In the All posts page, find the draft post and via Quick Edit, publish it
  • Go to /post-sitemap.xml sitemap and find that post
  • Confirm that the Last Mod. value for that post reflect the time you published it

Advanced settings in post metabox:

  • Create a Contributor and with that user go to add a post
  • Confirm that you don't get the Schema tab in the Yoast metabox
  • From your admin account, disable the Restrict advanced settings for authors setting (Site basics->Site preferences->Restrict advanced settings for authors setting)
  • With the contributor account refresh the edit post page
  • Confirm now that you do get the Schema tab in the Yoast metabox.

XML sitemap redirect:

  • Make sure you have XML sitemaps enabled in your Yoast SEO settings
  • Set your number of posts for your sitemap suitably low using the wpseo_sitemap_entries_per_page filter (basically it should be more than your number of posts, so as to have multiple post sitemaps:
add_filter( 'wpseo_sitemap_entries_per_page', function () { return 10; } );
  • Go to your sitemap index (http://example.com/sitemap_index.xml).
  • The first sitemap entry for each content type should not have a number.
  • If you visit, for example, /post-sitemap1.xml, you should be redirect to the sitemap without a number.
  • Now disable XML sitemaps enabled in your Yoast SEO settings.
  • Go to http://example.com/sitemap_index.xml and confirm you get a 404

Upgrade routine:

  • In your inc\class-upgrade.php file, add the '23.8-RC0' => 'upgrade_238', line, after the '22.6-RC0' => 'upgrade_226', line and also the
private function upgrade_238() {
	error_log('upgrade_238');
}

function after the upgrade_226() function

  • Perform an upgrade routine via the test helper and confirm in your debug.log that you get the upgrade_238 log. Refresh the admin a couple of times and confirm that you don't get another upgrade_238 log.
  • Check the wpseo option and confirm that the version and previous_version settings have an appropriate value.

Cornerstone filter:

  • Install Premium and make sure you have the cornerstone feature enabled in your Yoast settings
  • Mark a post as cornerstone and in the posts page, enable the filter to display only the cornerstone posts
  • Confirm that you get only the post marked as cornerstone
  • Also confirm that when you type yoastFilterExplanation in the console, you get the translated version of the Mark the most important post as 'cornerstone content' to improve your site structure. Learn more about cornerstone content string
  • Disable the feature and go again to the filtered post page URL (http://example.com/wp-admin/edit.php?yoast_filter=cornerstone&post_type=post)
  • Confirm that you now get all posts

Video content width:

  • Install Video and got to its settings
  • Add a value in Content width setting and after a refresh confirm that the value is stored.

Youtube embeds:

  • In Video SEO settings, enable the YouTube embeds: make pages load faster by only loading the YouTube player when the user clicks play setting
  • Go to the frontent of the site, go to the page source, search for videoSEOGenerateYouTubeThumbnail and confirm you find it in a script in the <head> of the page
  • Disable that settings, reload the frontend and confirm that you no longer get that script.

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

  • This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

  • I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

  • I have tested this code to the best of my abilities.
  • During testing, I had activated all plugins that Yoast SEO provides integrations for.
  • I have added unit tests to verify the code works as intended.
  • If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
  • I have written this PR in accordance with my team's definition of done.
  • I have checked that the base branch is correctly set.

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label.
  • I have added my hours to the WBSO document.

Fixes https://github.com/Yoast/plugins-automated-testing/issues/1871

@leonidasmi leonidasmi force-pushed the 1871-wp-67-function-load_plugin_textdomain-was-called-incorrectly-alternative branch from 10e675b to 9b570e3 Compare October 24, 2024 10:02
@coveralls
Copy link

coveralls commented Oct 24, 2024

Pull Request Test Coverage Report for Build 704a401668a6b5dbc72d3ea7e6f22c1de0a99e16

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 22 of 50 (44.0%) changed or added relevant lines in 12 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-2.6%) to 54.582%

Changes Missing Coverage Covered Lines Changed/Added Lines %
admin/class-admin-init.php 0 1 0.0%
admin/metabox/class-metabox.php 0 1 0.0%
inc/class-upgrade.php 1 2 50.0%
inc/class-wpseo-meta.php 0 1 0.0%
src/helpers/options-helper.php 0 2 0.0%
wp-seo-main.php 0 4 0.0%
admin/filters/class-abstract-post-filter.php 0 5 0.0%
admin/class-admin.php 0 6 0.0%
inc/class-rewrite.php 4 11 36.36%
Totals Coverage Status
Change from base Build f61227287fbfdf8c175ddd2168336ac22dfd2540: -2.6%
Covered Lines: 29675
Relevant Lines: 54634

💛 - Coveralls

@leonidasmi leonidasmi changed the title 1871 wp 67 function load plugin textdomain was called incorrectly alternative Call _load_textdomain_just_in_time correctly Oct 25, 2024
igorschoester and others added 18 commits October 25, 2024 10:33
* override the WP button style "white-space: nowrap" with "white-space: normal"
* rest is apply difference to align the comment and an auto-format
Looks better, approved by Kai
And while Kai was looking: spacing should be 24px instead of 32px
…possible and do that for setting the version options
@leonidasmi leonidasmi changed the base branch from release/23.8 to trunk October 25, 2024 10:00
@leonidasmi leonidasmi force-pushed the 1871-wp-67-function-load_plugin_textdomain-was-called-incorrectly-alternative branch from 314d249 to b28111a Compare October 25, 2024 10:02
@leonidasmi leonidasmi changed the base branch from trunk to release/23.8 October 25, 2024 10:02
@leonidasmi leonidasmi changed the base branch from release/23.8 to trunk October 25, 2024 10:02
@leonidasmi leonidasmi added the changelog: other Needs to be included in the 'Other' category in the changelog label Oct 25, 2024
@leonidasmi leonidasmi marked this pull request as ready for review October 29, 2024 14:57
Copy link
Member

@pls78 pls78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR: ✅

@vraja-pro
Copy link
Contributor

ACC and impact checks ✅

@vraja-pro vraja-pro merged commit fefa3a7 into trunk Nov 4, 2024
23 checks passed
@vraja-pro vraja-pro deleted the 1871-wp-67-function-load_plugin_textdomain-was-called-incorrectly-alternative branch November 4, 2024 11:25
@leonidasmi leonidasmi added this to the 23.9 milestone Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: other Needs to be included in the 'Other' category in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants