1501259 - editing a plaintext draft, all lines displayed in blue (in quote span)

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[ruediger]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

typing/answering a message, sent it to draft, go to draft, edit the message again.


Actual results:

All lines in the message are displayed as Quote, even the lines I typed and also Thunderbird now handles all new lines that I type into the message as quote, for example at word-wrap and color.


Expected results:

The message should not change by sending it to draft.

Comment 1

[WaltS48 [:walts48]]

I can't reproduce using Mozilla/5.0 (X11; Linux x86_64; rv:64.0) Gecko/20100101 Thunderbird/64.0a1 BuildID: 20181022100143 or my Thunderbird 64.0a1 on Windows 10.

Does the problem occur if you use safe mode?

Comment 2

[Jorg K (CEST = GMT+2)]

Which version are you using?

Comment 3

[ruediger]

(In reply to Jorg K (GMT+2) from comment #2)
> Which version are you using?

Actual Windows 64 trunk 64.0a1 (2018-10-22) (64-bit) nightly 20181022100143, but the problem is already some month old.
I reported it on a german newsgroup on August the 26 with a nightly from 63.0a

It seems, the problem only appears in plain-ascii-mails/postings and not in html(sorry, I forgot to tell this).

Comment 4

[ruediger]

(In reply to WaltS48 [:walts48] from comment #1)
> I can't reproduce using Mozilla/5.0 (X11; Linux x86_64; rv:64.0)
> Gecko/20100101 Thunderbird/64.0a1 BuildID: 20181022100143 or my Thunderbird
> 64.0a1 on Windows 10.
> 
> Does the problem occur if you use safe mode?

Yes.

Comment 5

[WaltS48 [:walts48]]

Tested with plain text newsgroups posts and still can't reproduce.

Tried a new profile?

Comment 6

[Jorg K (CEST = GMT+2)]

Confirmed.

STR:
Do a plain-text replay to an e-mail, usually Shift+Reply if your default is HTML.
Note some quoted text in blue.
Add some new text, should be black.
Save and close draft.
Look at the draft, it looks OK, with a vertical quotation bar.
Now edit the draft. It comes back all blue, not the way it was saved.
Sadly ThunderHTMLedit doesn't work in TB 63/64/65, so we can't see what's going on.

Works OK in TN 60 ESR.

Alice, can you please find the regression for us.

Comment 7

[Jorg K (CEST = GMT+2)]

Looking at the bad re-edited draft in the Inspector, I can see that the entire message is now wrapped into the "quote span"
  <span style="white-space: pre-wrap; display: block; width: 98vw;">
instead of just the quote part. Hmm, not good.

Comment 8

[Alice0775 White]

I can only reproduce if Add some new text at the bottom of reply plain text e-mail.

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=6181f7c5393bd19d9555922d68f4fede6a654714&tochange=a08367c1e5464a339016807c5553067b67e0cb2a
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=54f69f217483467dbd22e02ab4a98d8d8bc0ded8&tochange=dd386b5b9fa7f5cd6dc4bbbfa0503b3eb2969af5

Suspect: Bug 1476897

Comment 9

[Jorg K (CEST = GMT+2)]

Thank you, Alice. Yes, I was adding new text below the quote.

Yes, something has gone wrong in bug 1476897.

When editing a plaintext draft - I think - we collect all the lines starting with ">" and insert those as quotation, then insert the unquoted lines.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, here could be a regression point:
https://searchfox.org/mozilla-central/rev/a21ac3d8e53cd37b6725f982b185d92fe47d8835/editor/libeditor/HTMLEditorDataTransfer.cpp#2021-2023,2026

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: bug1501259-1.diff

Could you check whether this patch fixes this bug?

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ac65b4fcaead5ef0c1798c702e1e7d24ae82f53

Comment 13

[Jorg K (CEST = GMT+2)]

Sorry, the patch doesn't apply. You appear to have other patches from bug 1465702 before in your queue:
https://hg.mozilla.org/try/graph/1ac65b4fcaead5ef0c1798c702e1e7d24ae82f53

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: bug1501259-1.diff

Oops, really sorry. Rebased.

Comment 15

[Jorg K (CEST = GMT+2)]

Comment on attachment 9020600 [details] [diff] [review]
bug1501259-1.diff

Thank you, this works fine, checked visually and with the Inspector.

Are these warnings relevant?
[8524, Main Thread] WARNING: HTMLEditRules::BeforeEdit() failed to handle someth
ing: 'NS_SUCCEEDED(rv)', file c:/mozilla-source/comm-central/editor/libeditor/HT
MLEditor.cpp, line 3612
[8524, Main Thread] WARNING: Perhaps, <br> element position has been moved to di
fferent point by mutation observer: 'currentPoint == pointToInsert', file c:/moz
illa-source/comm-central/editor/libeditor/HTMLEditRules.cpp, line 1555

The second one repeats about 10 times.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thanks!

(In reply to Jorg K (GMT+2) from comment #15)
> Are these warnings relevant?
> [8524, Main Thread] WARNING: HTMLEditRules::BeforeEdit() failed to handle
> someth
> ing: 'NS_SUCCEEDED(rv)', file
> c:/mozilla-source/comm-central/editor/libeditor/HT
> MLEditor.cpp, line 3612
> [8524, Main Thread] WARNING: Perhaps, <br> element position has been moved
> to di
> fferent point by mutation observer: 'currentPoint == pointToInsert', file
> c:/moz
> illa-source/comm-central/editor/libeditor/HTMLEditRules.cpp, line 1555
> 
> The second one repeats about 10 times.

I've seen those warnings a lot in these days. Although I need to investigate it, but I keep working on urgent bugs...

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1501259 - HTMLEditor::InsertAsPlaintextQuotation() shouldn't refer |newNode| after calling its |forget()|

Regression of bug 1476897, HTMLEditor::InsertAsPlaintextQuotation() refers
|newNode| which is new created <span> element by the method at collapsing
Selection.  However, it's already been moved to its out param.  Therefore,
after the fix of bug 1476897, it won't collapse Selection.  This patch
moves the code moving |newNode| to the out param after collapsing Selection.

Additionally, this makes HTMLEditor::InsertAsCitedQuotationInternal() use
same style code around collapsing Selection since it's also tested by the
new test of nsIEditorMailSupport.insertAsCitedQuotation().

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/870be0de691a
HTMLEditor::InsertAsPlaintextQuotation() shouldn't refer |newNode| after calling its |forget()| r=m_kato

Comment 19

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/870be0de691a