Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#53589 closed defect (bug) (fixed)

Twenty Twenty One: Translation file loaded twice

Reported by: chouby's profile Chouby Owned by: audrasjb's profile audrasjb
Milestone: 6.0 Priority: normal
Severity: normal Version: 5.6
Component: Bundled Theme Keywords: has-patch commit assigned-for-commit
Focuses: performance Cc:

Description

Steps to reproduce:

  1. Setup a WordPress install with only Twenty Twenty One.
  2. Add the plugin Query Monitor
  3. Visit the front page and click on the Languages item of the Query Monitor admin bar menu.
  4. Check that the Twenty Twenty One translation file is loaded twice.

Analysis:
The first time, the translation file is loaded by _load_textdomain_just_in_time() due to a call to __() in https://github.com/WordPress/wordpress-develop/blob/5.7.2/src/wp-content/themes/twentytwentyone/inc/block-patterns.php#L20

The second time, the translation file is loaded by a direct call to load_theme_textdomain() in https://github.com/WordPress/wordpress-develop/blob/5.7.2/src/wp-content/themes/twentytwentyone/functions.php#L36

Attachments (2)

Bildschirmfoto 2021-12-29 um 21.50.01.png (20.2 KB) - added by zodiac1978 2 years ago.
Screenshot of the twice loaded translation file
53589.diff (38.9 KB) - added by zodiac1978 2 years ago.
Wrapping the registration of pattern and pattern category in a function hooked to init

Download all attachments as: .zip

Change History (14)

#1 @desrosj
3 years ago

  • Keywords needs-testing added
  • Summary changed from Twenty Twenty One translation file loaded twice to Twenty Twenty One: Translation file loaded twice

@zodiac1978
2 years ago

Screenshot of the twice loaded translation file

#2 @zodiac1978
2 years ago

Confirmed for WordPress 5.8.2 and WordPress 5.9-beta4-52420.
Problem is still there.

Not sure about the triggering call, but there is something triggering the translation before the actual translation setup is happening.

#3 @zodiac1978
2 years ago

I can confirm that the line 20 in /inc/block-patterns.php is triggering the first loading of the translation file:

<?php
require get_template_directory() . '/inc/block-patterns.php';

But fixing only this line wouldn't be enough. There are many more occurrences of translation function calls in this file. The problem is, that the file is executed immediately on loading. The similar file /inc/block-styles is using an action to execute the code:

<?php
add_action( 'init', 'twenty_twenty_one_register_block_styles' );

According to this comment (https://developer.wordpress.org/reference/functions/register_block_pattern/#comment-4675) and to the docs (https://developer.wordpress.org/block-editor/reference-guides/block-api/block-patterns/#register_block_pattern) this should also happen on the init hook.

@zodiac1978
2 years ago

Wrapping the registration of pattern and pattern category in a function hooked to init

#4 @zodiac1978
2 years ago

  • Keywords has-patch dev-feedback added; needs-testing removed

After patch the patterns still show up, but the translation is only loaded once and for twenty_twenty_one_setup() as the calling function.

Ready to review.

Last edited 2 years ago by zodiac1978 (previous) (diff)

#5 @sabernhardt
2 years ago

  • Milestone changed from Awaiting Review to 6.0

#6 @zodiac1978
2 years ago

Hi @sabernhardt - could you explain to me why you milestoned this for 6.0. The bundled themes could be updated independently from core updates, or not? So why do we wait to fix it? There were several theme updates in the meantime. Any reason for this? Just curious. :)

#7 @sabernhardt
2 years ago

Even when bundled themes are updated separately from the WordPress versions, the ticket has a milestone according to the WP (minor) version number. I did not find an obvious reason to rush this change before the major release, but if it's ready sooner, committers can move the milestone earlier.

#9 @audrasjb
2 years ago

  • Keywords commit assigned-for-commit added; dev-feedback removed
  • Owner set to audrasjb
  • Status changed from new to accepted

Looks good to me and tests are passing in the above PR. Self assigning for commit.

#10 @audrasjb
2 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 53121:

Twenty Twenty One: Prevent loading translation file twice.

This changes wraps the registration of patterns and pattern categories in a function hooked to init, to avoid loading the Twenty Twenty One translation file twice.

Props zodiac1978, sabernhardt, audrasjb.
Fixes #53589.

#11 @audrasjb
2 years ago

I forgot chouby in the props list, sorry. I added the props manually on the Make/Core manager.

Note: See TracTickets for help on using tickets.