I18N: Read the correct revision date key from .l10n.php files - #12844
I18N: Read the correct revision date key from .l10n.php files#12844jigneshbhavani wants to merge 1 commit into
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
irozum
left a comment
There was a problem hiding this comment.
Good catch — wp_get_l10n_php_file_data() was still only looking for po-revision-date, but GlotPress renamed that key to translation-revision-date a while back, so PO-Revision-Date silently comes back empty for any current language pack that lacks a sibling .po file, which then causes wp_get_installed_translations() to treat an up-to-date pack as stale on every update check. The fix (checking translation-revision-date first, falling back to po-revision-date) is correctly scoped — I confirmed wp_get_l10n_php_file_data() has a single caller in core (wp_get_installed_translations()), so there's no back-compat surface beyond this file.
I ran the l10n and i18n groups plus PHPStan — all clean, and the counts (267/268 tests, 808/813 assertions, 0 failures) match what's in the PR description. I also grepped for other references to po-revision-date/translation-revision-date in src/ and tests/ and didn't find anything else that would need updating in step with this.
One small thing worth a sentence in the description (not blocking): POT-Creation-Date is left mapped only to pot-creation-date, which per your own table doesn't appear in either fixture format, so it's presumably always empty too — you note this is a separate decision, which seems right, but it might be worth a quick follow-up ticket so it doesn't get lost.
swissspidy
left a comment
There was a problem hiding this comment.
Turns out this is a fallacy, the header in tests/phpunit/data/languages/de_CH.l10n.php was simply wrong from the beginning. There was never any renaming.
Thus, this PR does way too much.
All we need is update the fixture file and wp_get_l10n_php_file_data to use the correct name. No need for new fixtures, tests, comments, or array handling.
|
Agreed, thanks for catching it. There was no rename. @irozum, the rename claim in your review came from my description and is wrong. Flagging it so it does not travel further. Reducing to the two changes you listed: the correct key in |
wp_get_l10n_php_file_data() looked for `po-revision-date`, which no generated file uses. The de_CH test fixture had the same wrong key. Props bejignesh. Fixes #65809.
9fef42e to
aa7d30f
Compare
wp_get_l10n_php_file_data()mapsPO-Revision-Dateontopo-revision-date. No generated.l10n.phpfile uses that key, the revision date is written astranslation-revision-date, soPO-Revision-Datealways comes back empty.tests/phpunit/data/languages/de_CH.l10n.phphas the same wrong key, which is whyTests_L10n::test_wp_get_installed_translations_for_core()passed. The fixture matched the code rather than a real file.tests/phpunit/data/l10n/plural-complex.php, added earlier in [57518], uses the correct key.Why it matters
wp_get_installed_translations()falls back to this reader when a translation has no sibling.pofile, and that value goes to api.wordpress.org to decide which language packs are stale. With an empty revision date, a pack that is already installed and current gets offered again.Measured on trunk with the Twenty Twenty-Five nl_NL pack, freshly installed, changing nothing but whether the
.pois on disk:wp_get_installed_translations()reportswp_update_themes().popresent2025-08-05 20:37:56+0000.poabsent''Testing
With the fixture corrected and the reader left unchanged,
test_wp_get_installed_translations_for_core()fails with''at the de_CH assertion, so the existing coverage guards this. No new test needed.--group l10n: 266 tests, 804 assertions, 0 failures.--group i18n: 267 tests, 809 assertions, 0 failures.Tests_Locale::test_get_weekday_undefined_indexPHPUnit 10 deprecation warning, identical on unmodified trunk.POT-Creation-Datemaps topot-creation-date, which no format writes, so it stays empty. Left alone here.An earlier revision of this PR claimed the key had been renamed upstream and carried a fallback array, a new fixture and a new test on the back of that. The rename claim was wrong, see the comments below, and the patch is now the two key corrections only.
Trac ticket: https://core.trac.wordpress.org/ticket/65809
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Drafting the patch and this description. I measured the offer counts on the rendered update check with and without the
.popresent, and confirmed the existing test fails with the fixture corrected and the reader unchanged. The earlier claim that the key had been renamed upstream was mine and I published it without verifying it against core's own history or the other fixture in the same suite. I take responsibility for that and for this result.