Problem/Motivation
1. social_swiftmail produces some warnings when using PHP 8.1 based on "Passing null to non-nullable internal function parameters is deprecated"
2. function social_swiftmail_preprocess_swiftmailer tries to get a border radius based on:
$border_radius = Xss::filter(theme_get_setting('border_radius', $theme_id));
As far as I see nad know a setting border_radius does not exits in socialbase or socailblue and should be replaced by "card_radius"
Steps to reproduce
Setup Open Social with PHP 8.1, make sure social_swiftmail is enabled and send emails using SwiftMail.
Proposed resolution
Check that value is not null before continue processing
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 3342642-social-swiftmail-wrong-param.patch | 1.32 KB | socialnicheguru |
| #2 | 3342642_social_swiftmail.patch | 1.72 KB | slowflyer |
Issue fork social-3342642
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
slowflyer commentedComment #3
liam morlandComment #4
donquixote commentedI don't like `!= null`. It obfuscates the type conversions. See https://3v4l.org/fL8Dp
E.g. `'' == null`, but `'' !== null`.
Either we only want to detect NULL, then we use `=== NULL` or `!== NULL`.
Or we want to detect false-ish values, then we could use `if ($v)` or `if (!$v)` or `if ((bool) $v)`.
(We could also use `empty()` but this hides undefined variables.)
Also NULL would be uppercase with Drupal coding standards. And there is a missing space after first `if`.
Then other instances of 'border_radius' also need to be replaced.
E.g. here in the same function social_swiftmail_preprocess_swiftmailer():
As far as I can see, setting this in variables will have no effect. But then again, I don't see either border_radius nor card_radius in the swiftmailer template. Maybe I am missing something?
----
I see one place where 'card_radius' is used when generating in-page CSS, in `improved_theme_settings_page_attachments()`.
But this has nothing to do with swiftmailer.
Comment #5
donquixote commentedBtw a shortcut would be this:
It would mean that the Xss::filter() is still executed even for zero, but I think it is acceptable.
I am putting here '0' (string) and not 0 (integer) because the filter function will convert it to string anyway.
But perhaps all of this is just a leftover that needs to be cleaned up?
Comment #6
donquixote commentedFor the `$disabled_greeting_keys`:
I assume in either case we want to ignore these empty string parts.
In that case, I assume we want to treat it the same as if it is NULL, so that the heading is always shown.
I say all of this from looking at the code, I did not actually test anything.
Btw the control flow in this part of the function is a bit cluttered and redundant, and the different parts are not in an ideal order.
It could be refactored like this:
Note that:
$social_swiftmail_config->get("disabled_user_greeting_keys") ?? ''.isset($message['key'])with!empty($message['key'])to also cover empty string. But we have to look the possible empty-ish values this might have, and what the consequences of that would be.The main point was to move this stuff down because it can be skipped if any of the conditions above fail.
Comment #7
kashandarash commentedhello, there is PR for this with two border-radius https://github.com/goalgorilla/open_social/pull/3482/files
Comment #8
kashandarash commentedComment #9
socialnicheguru commentedUploaded a patch based on PR that applies to 11.9.x
Comment #13
tbsiqueira