MailExtension getComposeDetails() returns plainTextBody with no trailing spaces and redundant leading spaces since change b96d9df1eacc2092114ac04dcbed3a6cc4612043
Categories
(Thunderbird :: Add-Ons: Extensions API, defect)
Tracking
(thunderbird_esr102+ fixed, thunderbird106 affected)
People
(Reporter: frederick888, Assigned: john)
Details
Attachments
(1 file)
|
48 bytes,
text/x-phabricator-request
|
wsmwk
:
approval-comm-esr102+
|
Details | Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:106.0) Gecko/20100101 Firefox/106.0
Steps to reproduce:
Calling messenger.compose.getComposeDetails(tab.id)
The compose window is in plain text mode, and the original text is
123456␣
␣Hello␣
(␣ denotes a regular space.)
Actual results:
plainTextBody from the returning value has no trailing spaces and redundant leading spaces:
123456
␣␣Hello
Expected results:
Prior to b96d9df1eacc2092114ac04dcbed3a6cc4612043, we used editor.outputToString("text/plain", Ci.nsIDocumentEncoder.OutputRaw) as plainTextBody.
(In Thunderbird 102.3.0 console:)
temp0.IsHTMLEditor()
false
editor = temp0.GetCurrentEditor()
editor.outputToString('text/plain', Ci.nsIDocumentEncoder.OutputRaw)
"123456␣
␣Hello␣
"
After this change we started using MsgUtils.convertToPlainText(htmlBody, true) instead:
htmlBody = trimContent(editor.outputToString('text/html', Ci.nsIDocumentEncoder.OutputRaw))
"<html><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"><style></style></head><body style="font-family: -moz-fixed; white-space: pre-wrap;">123456 <br> Hello <br></body></html>"
parserUtils.convertToPlainText(htmlBody, Ci.nsIDocumentEncoder.OutputPersistNBSP | Ci.nsIDocumentEncoder.OutputFormatted | Ci.nsIDocumentEncoder.OutputDisallowLineBreaking | Ci.nsIDocumentEncoder.OutputFormatFlowed, 990)
"123456
␣␣Hello
"
parserUtils.convertToPlainText(htmlBody, Ci.nsIDocumentEncoder.OutputPersistNBSP | Ci.nsIDocumentEncoder.OutputFormatted | Ci.nsIDocumentEncoder.OutputDisallowLineBreaking, 72) // with formatFlowed = false, there are no more redundant leading spaces but trailing ones are still removed
"123456
␣Hello
"
IMHO when composing in plain text, plainTextBody should be exactly the same as what the user has in the compose window editor.
|
Assignee | |
Comment 1•3 years ago
|
||
See Bug https://bugzilla.mozilla.org/show_bug.cgi?id=1692439#c3 and the following comments. We will not be able to revert, but I have a look at the strange double space. The trimmed trailing space can probably be reverted by removing the outer trim.
|
Reporter | |
Comment 2•3 years ago
|
||
Thank you for the quick response, John!
A user reported https://github.com/Frederick888/external-editor-revived/issues/61 to me and I really appreciate the help :)
|
|
Assignee | |
Comment 3•3 years ago
•
|
||
Ok, the request is to return the content of TEXT composers "as-is", but I think we should keep unifying the line ending. IIRC, we always return \n line ending regardless of the used OS. I would like to keep doing that.
The proposed way to fix this is:
let plainTextBody = composeWindow.IsHTMLEditor()
? trimContent(MsgUtils.convertToPlainText(body, true))
: parserUtils.convertToPlainText(body, Ci.nsIDocumentEncoder.OutputLFLineBreak, 0);
So we are not changing the return value for HTML composers and keep doing what we are currently doing. For TEXT composer, we could directly use parserUtils.convertToPlainText() but without most flags (MsgUtils.convertToPlainText() is using this as well, but with a different set of flags).
Objections?
|
|
Assignee | |
Comment 4•3 years ago
|
||
|
||
Updated•3 years ago
|
|
|
Assignee | |
Comment 5•3 years ago
|
||
Here is a try run:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=e639fce92fd81795b9c9a5fb919c75abbe014e4f
From the green Ba items you can download a built (Artifacts and Debug Tools tab -> target.zip / target.tar.bz2) to test the patch.
|
|
Reporter | |
Comment 6•3 years ago
|
||
@John Thank you for the fix!
but I think we should keep unifying the line ending
That doesn't really concern me since I always convert the body to CRLF in my extension :)
Though one issue that I noticed is that there's now a new blank line added to plainTextBody due to an extra <br> in the HTML:
Original text:
123456␣
␣Hello␣
Tested in both 102.3.0 and the test build:
$ body = trimContent(editor.outputToString("text/html", Ci.nsIDocumentEncoder.OutputRaw))
"<!DOCTYPE html>
<html><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"><style></style></head><body style="font-family: -moz-fixed; white-space: pre-wrap;">123456 <br> Hello <br></body></html>"
$ JSON.stringify(parserUtils.convertToPlainText(body, Ci.nsIDocumentEncoder.OutputLFLineBreak, 0))
""123456 \n Hello \n""
$ compared with the old implementation
""123456 \n Hello ""
|
|
Reporter | |
Comment 7•3 years ago
|
||
Sorry, the 'old implementation' output was from
JSON.stringify(editor.outputToString('text/plain', Ci.nsIDocumentEncoder.OutputRaw))
|
|
Assignee | |
Comment 8•3 years ago
•
|
||
It looks like we can simply remove the last line break, if there is one. If you intentionally added a line break at the end, there will be two. I update the tests to cover this.
https://hg.mozilla.org/try-comm-central/rev/6ad7ee025f9f6f6af5a73b180521a7a3c01fbd1c#l1.42
I think we are good for check-in:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&collapsedPushes=1128357%2C1128350%2C1128348%2C1128024%2C1128395%2C1128394&revision=a3e5d92893271dd4c811a307e86d237cc05c6c79
|
|
Reporter | |
Comment 9•3 years ago
|
||
Yup that LGTM. Thanks again :)
|
||
Comment 10•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/42872bef2ab8
Return plain text content as-is. r=mkmelin
|
||
Updated•3 years ago
|
|
|
Assignee | |
Comment 11•3 years ago
|
||
Comment on attachment 9296467 [details]
Bug 1792551 - Return plain text content as-is. r=mkmelin
[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
Wrongly returned spaces and newlines.
Testing completed (on c-c, etc.):
On Beta for more than 30 days.
Risk to taking this patch (and alternatives if risky):
Low.
|
||
Comment 12•3 years ago
|
||
Comment on attachment 9296467 [details]
Bug 1792551 - Return plain text content as-is. r=mkmelin
[Triage Comment]
Approved for esr102
|
||
Comment 13•3 years ago
|
||
| bugherder uplift | ||
Thunderbird 102.5.1:
https://hg.mozilla.org/releases/comm-esr102/rev/b251c831969c
Description
•