Bug #19415
closedAudit usage of custom en_CAC locale
0%
Description
In our codebase, we use a custom en_CAC
locale to load these strings:
1. WordPress - https://github.com/cuny-academic-commons/cac/blob/12e3001973274946faa69cd37360a2d7fce35f04/wp-content/plugins/bp-custom.php#L492-L509
2. BuddyPress - https://github.com/cuny-academic-commons/cac/blob/12e3001973274946faa69cd37360a2d7fce35f04/wp-content/plugins/bp-custom.php#L172-L176
3. BP plugins - https://github.com/cuny-academic-commons/cac/blob/12e3001973274946faa69cd37360a2d7fce35f04/wp-content/plugins/bp-custom.php#L1837C4-L1865
For point 1, en_CAC
is not a valid locale and can cause issues when using certain javascript libraries like Shoelace that attempts to load a locale file for CAC
. Also, most of the strings in our custom WordPress localization file are no longer being used. I would recommend removing the update_option( 'WPLANG', 'en-CAC' )
approach and doing string overrides using the 'gettext_default'
filter on a page-by-page basis. It looks like the only string we would need to override is the one for /wp-admin/ms-delete-site.php
: https://github.com/cuny-academic-commons/cac/blob/12e3001973274946faa69cd37360a2d7fce35f04/wp-content/languages/en_CAC.po#L34-L38 .
Points 2 and 3 are not as important to address, but it looks like we only made a few changes overall: https://github.com/cuny-academic-commons/cac/commits/master/wp-content/languages . I think we need to audit this list to determine whether most of our string overrides are even necessary nowadays.
Updated by Boone Gorges 11 months ago
Thanks for summarizing the situation, Ray. The introduction of en_CAC
was me trying to do things "the right way". But if it's not the right way, let's tear it out.
As for the other language files, at a glance it looks like very few of the overridden strings are actually in use anyway. The only thing would be the use of the word 'Files' where BP Group Documents uses 'Documents'. But even there, we override much of this interface with bp-group-library and/or our own templates. So I kinda think we should just tear out the entire directory, then do a quick review of group files, and swap out strings in the template files as necessary.
Updated by Raymond Hoh 11 months ago
I've created a new branch -- 19415-remove-CAC-locale -- and have removed all language files related to en_CAC
.
I've re-implemented the string override for /wp-admin/ms-delete-site.php
here: https://github.com/cuny-academic-commons/cac/commit/f9c701354be17e21d82f1ff16402cd9375b9b48e#diff-e199d5f24d983e8d01b0190e1d59fc65c9cd3659c2841dd6664a47fdd4469f00, https://github.com/cuny-academic-commons/cac/commit/f9c701354be17e21d82f1ff16402cd9375b9b48e#diff-7513ab4360dfc40429ee4da6a1b26616a8bd52cef26a095f0f905ebc7cb72a81 .
For older sites with the 'en_CAC'
locale as set for their WPLANG
option, I've added a filter to set the locale to 'en'
in https://github.com/cuny-academic-commons/cac/commit/4b87c545e9db26a7c85f39f7b5e3b5c01900e5c1 .
As for:
But even there, we override much of this interface with bp-group-library and/or our own templates. So I kinda think we should just tear out the entire directory, then do a quick review of group files, and swap out strings in the template files as necessary.
I did a quick overview of the Group Library interface and I didn't notice anything that popped up immediately string-wise (I did spot a display bug when on a doc page, see #19436), but let me know if you encounter anything, Boone. I think we can consider doing this for v2.3.0, but let me know if we want to push this back a bit.
Updated by Boone Gorges 11 months ago
- Target version set to 2.3.0
I did a quick overview of the Group Library interface and I didn't notice anything that popped up immediately string-wise (I did spot a display bug when on a doc page, see #19436), but let me know if you encounter anything, Boone. I think we can consider doing this for v2.3.0, but let me know if we want to push this back a bit.
Yes, let's ship it for 2.3.0. Thank you for working on this!
Updated by Raymond Hoh 11 months ago
- Status changed from New to Staged for Production Release
Yes, let's ship it for 2.3.0. Thank you for working on this!
Great! I've merged 19415-remove-CAC-locale
branch into feature/17769-cv-editor
branch, which is the branch we are testing on cdev.
Updated by Boone Gorges 10 months ago
- Status changed from Staged for Production Release to Resolved