FlowCrypt / flowcrypt-browser

FlowCrypt Browser extension for Chrome and Firefox
https://flowcrypt.com
Other
373 stars 46 forks source link

Don't add "--" separator if user signature begins with "--" #5368

Closed loulan closed 1 year ago

loulan commented 1 year ago

FlowCrypt keeps adding "--" at the beginning of my signature. Which is unfortunate, because my signature already begins with "-- ". It's pretty ugly.

I believe correct behavior would be to not add anything to the signature?

sosnovsky commented 1 year ago

Yeah, we're adding -- in the beginning of signature to separate it from text content. For fixing this case we should add check, and if signature text already begins with -- then extension don't need to add additional -- separator. Thanks for reporting!

sosnovsky commented 1 year ago

@loulan we re-checked our implementation and found that -- shouldn't be added to your signature, if it's already starts with dashes --. Can you please share your signature, so we can reproduce your issue? Or maybe just it's structure - is it single line or multiline, does it use html or it's plain one? Thanks

loulan commented 1 year ago

My signature first has a newline, then two dashes and a space and a newline, and finally my name/website etc.

I.e., it's something like:

================== (below this line)

-- 
My name
My website
================== (above this line)
sosnovsky commented 1 year ago

Hmm, interesting - I tried your signature and default Gmail compose adds additional --:

Screenshot 2023-08-18 at 12 12 28

While FlowCrypt has single line with dashes:

Screenshot 2023-08-18 at 12 13 56

Does default Gmail compose box add additional -- for you too?

loulan commented 1 year ago

The default gmail box doesn't add an additional --, flowcrypt does.

Maybe one difference is that it's HTML, as there is a link for my website.

loulan commented 1 year ago

Here are some screenshots.

The signature in the settings:

Capture d’écran 2023-08-18 à 11 21 05

Gmail box:

Capture d’écran 2023-08-18 à 11 18 51

FlowCrypt box:

Capture d’écran 2023-08-18 à 11 19 18
martgil commented 1 year ago

Excuses,

Thanks @loulan I think I have found the issue. Could you please observe if there is an additional newline on the very top of the signature before the --. If so, could you please try removing all the newline all the way before the --?:

image

loulan commented 1 year ago

The signature starts with a newline, yes.

loulan commented 1 year ago

I have the same problem without the newline.

Capture d’écran 2023-08-18 à 11 33 47 Capture d’écran 2023-08-18 à 11 34 18 Capture d’écran 2023-08-18 à 11 34 28
sosnovsky commented 1 year ago

Do you have this checkbox checked in your signature settings?

Screenshot 2023-08-18 at 12 45 58
loulan commented 1 year ago

Oh wow, I hadn't noticed this option... Yes, it's checked.

sosnovsky commented 1 year ago

So now it's clear why -- isn't added in default compose box, but I still can't reproduce additional -- in FlowCrypt compose box.

Can you please send encrypted message with your signature (which has duplicated --) to flowcrypt.compatibility@gmail.com?

loulan commented 1 year ago

Done.

sosnovsky commented 1 year ago

Thanks! Checking it

sosnovsky commented 1 year ago

@loulan do you have Firefox browser? We have a newer version of extension available for it (will be updated for Chrome soon too) - https://s3-us-west-2.amazonaws.com/cryptup-release/firefox/cryptup-io-firefox-8.5.0.xpi. It'll be great if you can check it and see if duplicated -- still present there or it's fixed

loulan commented 1 year ago

I just tried with FlowCrypt 8.5.0 on Firefox and I have the same problem.

loulan commented 1 year ago

Interestingly, it seems that the issue disappears if the second line of my signature (after the initial newline) is just two hyphens and a new line, instead of two hyphens followed by a space and a newline.

I like keeping a space after the two hyphens though, as it is standard. See section "Standard delimiter" in: https://en.wikipedia.org/wiki/Signature_block

The Usenet news system standards say that a signature block is conventionally delimited from the body of the message by a single line consisting of exactly two hyphens, followed by a space, followed by the end of line (i.e., in C-notation: "-- \n").[9][10][11] This latter prescription goes by many names, including “dash dash space”, "sig dashes", "signature cut line", "sig-marker", "sig separator" and "signature delimiter". It allows software to automatically mark or remove the sig block as the receiver desires.

Most Usenet clients (including, for example, Mozilla Thunderbird) will recognize the signature block delimiter in a news article and will cut off the signature below it when inserting a quote of the original message into the composition window for a reply. Although the Usenet standards strictly apply only to Usenet news articles, this same delimiter convention is widely used in email messages as well, and email clients (such as K-9 and Opera Mail) commonly use it for recognition and special handling of signatures in email.

sosnovsky commented 1 year ago

I tried to reproduce, but it works well for me in both cases - with and without space after two hyphens.

Please also keep in mind that signature in FlowCrypt compose box isn't refreshed immediately after changing in settings, you probably need to reload page for fetching updated signature. So I also add some number (like Test signature 1, then Test signature 2 etc) to be sure that signature was refreshed.

loulan commented 1 year ago

I already do this, which is why there is "My name 2" in my screenshots above.

I'm really confused about why you can't reproduce this, it's very strange.

sosnovsky commented 1 year ago

Yeah, this is strange, as it works well for Mart too, so we can't find what needs to be fixed :)

Can you please go to FlowCrypt extension settings page, click local_store in footer and copy footer property from modal. So it'll look like this:

      "footer": "

-- 
My test signature2
"
Screenshot 2023-08-22 at 15 36 38
loulan commented 1 year ago

Can I put a breakpoint somewhere in the developer tools or something like that to understand why the branch that should detect the "--" is not taken?

sosnovsky commented 1 year ago

Ah, I was able to reproduce it by adding space to the first empty line, so my signature looks like this:

<space>
--<space>
signature

If first space is removed, then second separator not added:


--<space>
signature
sosnovsky commented 1 year ago

But for you it happens even without newline at the beginning of signature, so it should be some other reason

martgil commented 1 year ago

@loulan Pardon me for bothering you. May I see how the FlowCrypt browser extension interprets your email footer? You can check this by going to FlowCrypt settings and clicking on the 'local_store' link button. After that, please kindly share the footer here starting from "footer": " until the closing " is reached, and paste it here. Please feel free to replace all personal information with dummy data. Thanks in advance.

image

martgil commented 1 year ago

Ah, I was able to reproduce it by adding space to the first empty line, so my signature looks like this:

<space>
--<space>
signature

If first space is removed, then second separator not added:


--<space>
signature

For me I'm able to replicate it when the signature already starts at -- but with space:

image image
sosnovsky commented 1 year ago

For me I'm able to replicate it when the signature already starts at -- but with space:

Oh, nice, can you then please implement fix so second separator won't be added in this case?

loulan commented 1 year ago

Two points:

"footer": "

-- 
My name
[http://my.website.com/](moz-extension://03296453-483e-4d31-b81b-ad2fe3a5d9c0/chrome/settings/modules/\"http://my.website.com/\")
"

Note that there are some old e-mail addresses that I don't use in the JSON that have an empty footer (""). Not sure if it matters.

I really don't understand why you guys don't manage to reproduce this. Maybe it's because I'm on a Mac and newlines are different? But I'm not sure it's the case in web browsers. I can't really think of anything else.

martgil commented 1 year ago

Hi @loulan,

We have created a custom FlowCrypt extension for you, as we assumed there might be a non-printable space character in your email signature's first line. To test it out, kindly unzip the compressed file and load it into your browser. You can choose either of the two options that suits you.

For Google Chrome browser, follow these steps:

  1. Go to chrome:extensions
  2. Enable developer mode
  3. Click on the Load unpacked button
  4. Choose the chrome-consumer folder

For Mozilla Firefox browser, follow these steps:

  1. Go to about:addons
  2. Click on Extensions
  3. Click on the gear icon
  4. Select "debug addons"
  5. Click on "Load temporary add-on..."
  6. Locate the firefox-consumer folder and load the manifest.json file

Once the extension is installed, please set it up and check if the issue no longer persists in this custom build. If the issue still exists, please go to the FlowCrypt settings, click on local_store at the bottom of the page, open the web developer tools, go to the console, and share the data from the JavaScript console with us via email at human@flowcrypt.com. Alternatively, you can write us an encrypted email through https://flowcrypt.com/me/tom.

image

FlowCrypt browser extension for Google Chrome: chrome-consumer.zip FlowCrypt browser extension for For Mozilla Firefox: firefox-consumer.zip

Thank you for your time. -Mart

loulan commented 1 year ago

Thank you for your efforts.

It's still not working though. I sent you the output of JavaScript console by e-mail.

martgil commented 1 year ago

Hello @loulan, thank you for sharing the JavaScript console logs with us. I was able to determine that the issue was caused by a non-breaking space  , which has a URL encoded value of %C2%A0.

You may not necessarily need to test the second custom build but feel free to do so.

chrome-consumer.zip

martgil commented 1 year ago

This PR has been closed as resolved based on the proposed fix at #5412. Please feel free to re-open this GitHub issue if anyone experiences a similar issue.

loulan commented 1 year ago

I confirm that the new version of the module fixed my issue. Thanks a lot!

There is still something I don't understand though. I was unhappy about having an non-breaking space instead of a regular space in my signature, so I wanted to fix that. I tried copy/pasting my signatures from the Gmail settings into text files and dumping their contents with hexdump, and they all contained spaces, i.e., 0x20, not non-breaking spaces, i.e., 0xc2a0. If I manually enter a non-breaking space with option+space into a text file and hexdump it, I see 0xc2a0.

So how I end up with a non-breaking space in the local_store is a mystery!

sosnovsky commented 1 year ago

Probably text editors convert such symbols as non-breaking spaces to plain spaces when pasting from other sources, as they usually remove text formatting on paste.