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

Remove add_reasons_to_defaults() from general initialization #627

Merged
merged 5 commits into from
Nov 1, 2024

Conversation

2ndkauboy
Copy link
Member

The add_reasons_to_defaults() adds the translatable strings to the $default. Those reasons are only needed inside the send_mail_notification() method, which also calls the add_reasons_to_defaults() function. Therefore, the function call can be removed from the general init() function.

Fixes #625

@2ndkauboy 2ndkauboy requested a review from Zodiac1978 October 18, 2024 12:17
@swissspidy
Copy link
Contributor

I think this re-introduces #508.

  1. Go to the Antispam Bee settings page
  2. See warnings

Query Monitor screenshot:

Screenshot 2024-10-18 at 14 43 53

Translation-wise it looks fine though.

@2ndkauboy
Copy link
Member Author

@swissspidy great catch!

I first wondered on how/where the Antispam_Bee_GUI was able to get self::$defaults, but then I saw it: ... extends Antispam_Bee 🙈

Oh boy! Time to get this old code gone and v3 ready!

My new commit should fix it. It now loads the "reasons" in an init hook callback. Still too much, but it should resolve both issues.

The `add_reasons_to_defaults()` adds the translatable strings to the `$default`.
Those `reasons` are only needed inside the `send_mail_notification()` method,
which also calls the `add_reasons_to_defaults()` function. Therefore, the function
call can be removed from the general `init()` function.
…st_in_time()`

This function is calling the `load_plugin_textdomain()`, which eventually become
deprecated soon. Since this plugin is also using the "Text Domain" plugin header,
it is not necessary to call this function at all.

By removing this function, the `_load_textdomain_just_in_time()` function will be
used, when the first string needs to be translated.
Since we don't explicitly load the translation files anymore
this will make sure that the necessary function is there.
@2ndkauboy 2ndkauboy force-pushed the fix/625-load_textdomain-just-in-time branch from be234e8 to f083959 Compare November 1, 2024 14:39
Copy link

sonarqubecloud bot commented Nov 1, 2024

Copy link
Contributor

@stklcode stklcode left a comment

Choose a reason for hiding this comment

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

LGTM

@2ndkauboy 2ndkauboy merged commit 14cb746 into master Nov 1, 2024
11 checks passed
@2ndkauboy 2ndkauboy deleted the fix/625-load_textdomain-just-in-time branch November 1, 2024 14:58
@stklcode stklcode added this to the 2.11.7 milestone Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notice: Function _load_textdomain_just_in_time was called incorrectly
3 participants