deltachat / deltachat-node

Email-based instant messaging for Node.js.
GNU General Public License v3.0
45 stars 11 forks source link

Signature reverts to default when I set it to nothing #505

Closed hut closed 3 years ago

hut commented 3 years ago

Thank you

Jikstra commented 3 years ago

@hut thanks for reporting :)

@Simon-Laux can you try if this happens on android too? A quick search through the code didn't lead to any obvious wrong thing. Maybe it's a core thing?

In core this line looks a little bit suspicious: https://github.com/deltachat/deltachat-core-rust/blob/a88893f262d55336dc72206d46c879ef2457811b/src/config.rs#L176 But I don't know when value.is_some() is actually true...

Simon-Laux commented 3 years ago

can't reproduce on android, also hut wrote above:

Expected behavior: When I set the signature to an empty string, I expect no signature to be sent. This is the behavior on the android app as well.

r10s commented 3 years ago

i think, the magic happens in dc_set_config() - here, if you pass NULL or the default signature, the signature is set to NULL (None in rust-land). that results in dc_get_config() returning the default signature.

so, imu, for a removing the signature, you have to call dc_set_config(context, "") and not dc_set_config(context, NULL).

r10s commented 3 years ago

node converts "" to NULL at https://github.com/deltachat/deltachat-node/blob/4873b17b18fd72cc4cfd6665b6ebe5a6fb4b0190/src/module.c#L1401 - therefore the reset to the default signature.

Simon-Laux commented 3 years ago

When changing this that it accepts is null we need to make sure that all settings in desktop still work as expected, not that we introduce other bugs while fixing this.

r10s commented 3 years ago

yip, one should be careful with these kind of things.

nb: one higher level, NULL is converted to "": https://github.com/deltachat/deltachat-node/blob/2088a99561115c20fbf1f91a6f14fb2579e232c6/lib/deltachat.ts#L829

so, maybe remove the first or both guards and check carefully all explicit/implicit calls to dc_set_config() for possible side-effects.