fix(DB): added charset to DB Params

[degoya]

Fix Database Problem with Emojis like 🦉📌

Kirby was not able to display or save Emojis like 🦉📌 to the Database. The reason was the wrong charset, the problem is that the charset is'nt loaded to params from the config.

Fixes

loading charset from config to DB params

[afbora]

I like the new charset config. We have also one more charset config that for setting via params.

https://github.com/degoya/kirby/blob/db-fix/src/Database/Database.php#L569

But I can't decide which one we should to use utf8 or utf8mb4. utf8mb4 charset is supported for all mysql versions? We should update both usage to utf8 or utf8mb4.

[degoya]

I like the new charset config. We have also one more charset config that for setting via params.

https://github.com/degoya/kirby/blob/db-fix/src/Database/Database.php#L569

But I can't decide which one we should to use utf8 or utf8mb4. utf8mb4 charset is supported for all mysql versions? We should update both usage to utf8 or utf8mb4.

i would prefer utf8mb4 it is supported by MySQL 8.0. 28 and later, this is also the default for Word Press and other DB based CMS today. this should only noted in the docs and you are still able to use utf8 as a setting for a fallback. but in modern times this should not be needed IMHO.

[bastianallgeier]

@afbora I think you found the correct line to set this. If we set it there, the additional definition in the DB class could just return null as default and would then automatically fall back to the value set in the Database class.

[afbora]

You're right @bastianallgeier I think we should implement like that

$parts[] = 'charset=' . ($params['charset'] ?? Config::get('db.charset', 'utf8mb4'));

https://github.com/getkirby/kirby/blob/develop-minor/src/Database/Database.php#L569

[distantnative]

@bastianallgeier @afbora wouldn't the opposite make more sense? Keep the Config::get together with the other configs, add the fallback there and then remove any fallback here https://github.com/degoya/kirby/blob/db-fix/src/Database/Database.php#L569 as it won't be needed anymore?

[afbora]

But charset won't set if params argument passed and chartset not defined in params.

[distantnative]

But my understanding is that $params['charset'] would always be defined when we merge this PR, wouldn't it?

$params is

• https://github.com/degoya/kirby/blob/db-fix/src/Database/Database.php#L157 $options
• https://github.com/degoya/kirby/blob/db-fix/src/Database/Database.php#L115 $params
• https://github.com/degoya/kirby/blob/db-fix/src/Database/Db.php#L51-L54

[distantnative]

Ah sorry, I see your point. If someone sets explicit $params for Db then it could be missing.

[distantnative]

Then I would do

'charset'  => Config::get('db.charset', null)

https://github.com/degoya/kirby/blob/db-fix/src/Database/Db.php#L51

And

		$parts[] = 'charset=' . ($params['charset'] ?? 'utf8mb4');

https://github.com/degoya/kirby/blob/db-fix/src/Database/Database.php#L569

[bastianallgeier]

@distantnative yep, that's how I would solve it too. @degoya do you want to fix it in your PR or should we take over?

[bastianallgeier]

@degoya I've taken over and made the last changes. Thank you for your help. We will make sure to mention you in the release notes. #6153