Project

General

Profile

Actions

Bug #19415

closed

Audit usage of custom en_CAC locale

Added by Raymond Hoh 11 months ago. Updated 10 months ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Category name:
WordPress (misc)
Target version:
Start date:
2023-12-14
Due date:
% Done:

0%

Estimated time:
Deployment actions:

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.

Actions #1

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.

Actions #2

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.

Actions #3

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!

Actions #4

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.

Actions #5

Updated by Boone Gorges 10 months ago

  • Status changed from Staged for Production Release to Resolved
Actions

Also available in: Atom PDF