colorediffs icon indicating copy to clipboard operation
colorediffs copied to clipboard

Lines are trimmed when replying to selection

Open CendioOssman opened this issue 2 years ago • 4 comments

This is a very nice addon that makes it much easier to read mails with diffs in them, but it has one annoying bug:

If you select a portion to reply to, then whitespace gets trimmed and the quote in the reply gets garbled. This does not happen if you reply to the entire mail (without selecting anything).

As an example, selecting the following in the original mail:

@@ -57,7 +58,7 @@
         (unwilling, _) = self._get_handlers()
 
         imp = MagicMock()
-        gi = imp.return_value
+        imp.side_effect = lambda *a, **kw: attrgetter(a[0])(imp)

And then replying, gives the following new message without the addon:

On 6/15/22 15:40, Pierre Ossman wrote:
> @@ -57,7 +58,7 @@
>           (unwilling, _) = self._get_handlers()
>   
>           imp = MagicMock()
> -        gi = imp.return_value
> +        imp.side_effect = lambda *a, **kw: attrgetter(a[0])(imp)

But with the addon we get this:

On 15/06/2022 15:40, Pierre Ossman wrote:
> |@@ -57,7 +58,7 @@ (unwilling, _) = self._get_handlers() imp = 
> MagicMock() - gi = imp.return_value + imp.side_effect = lambda *a, **kw: 
> attrgetter(a[0])(imp)|

(note that there is some whitespace trimming in the "good" case as well, but that's a thunderbird bug)

CendioOssman avatar Jun 16 '22 06:06 CendioOssman

Interesting, thanks for the report! I don't usually use the selection feature, so didn't notice before.

I suppose that because the text is displayed with some HTML tags to get the colours, all of this gets trimmed when converted to plain text for the answer. I must confess I hadn't expected the change to be passed to the composition window. I wonder how we can avoid that :thinking: I wonder if there's a way to convert back to plain text when we select the text.

I don't have an immediate solution, I'll have to dive into it to find something when I have some time. I can't promise you a fix anytime soon :/. I note that the bug only affects plain-text answers at least, and selecting for an HTML reply seems to work fine.

Qeole avatar Jun 16 '22 12:06 Qeole

I'm assuming that plain-text is still HTML under the hood. So I would guess there is something magical that Thunderbird itself does there that needs to be replicated in the add-on. I don't know if the inspector can be used to see such details?

CendioOssman avatar Jun 16 '22 12:06 CendioOssman

The inspector would show me what the HTML code is for the highlighted message in the message panel, but I don't know if we can (or how to) use it in the composition window for the answer.

I would guess there is something magical that Thunderbird itself does there that needs to be replicated in the add-on.

Yes there's probably something happening when quoting only a fragment of the original message, but I don't know where to find this, we'll likely have to dive into the code eventually. Then once we find it, there will be the question of how fix it, because I don't think I can hook from the add-on on whatever happens with the selected text when the user clicks the “reply” button. But first we need to figure out what the missing bit is, I suppose.

Qeole avatar Jun 25 '22 22:06 Qeole

Found a small clue, if I change the containing <code> to a <pre>, then things improve. It now correctly quotes as long as I include something from outside the diff in the selection. However, if the selection only contains things from the diff, then things still break.

CendioOssman avatar Jun 27 '22 07:06 CendioOssman