Sending a blank line breaks OTR

RESOLVED FIXED in 1.6

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: arlolra, Assigned: arlolra)

Tracking

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Posted patch null.patch (obsolete) — Splinter Review
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.104 Safari/537.36

Steps to reproduce:

Sent a message with a blank line using the OTR extension.


Actual results:

IRC split the lines and the empty message fell back to original encrypted message when displaying the outgoing message.


Expected results:

Displayed the blank line.

Updated

5 years ago
Summary: null → Sending a blank line breaks OTR
Comment on attachment 8511139 [details] [diff] [review]
null.patch

This needs a comment, otherwise someone will look at the code, clean it up with || and re-introduce the bug.
Attachment #8511139 - Attachment is patch: true
Attachment #8511139 - Attachment mime type: text/x-patch → text/plain
(Assignee)

Comment 2

5 years ago
Posted patch null.patch from comment 1 (obsolete) — Splinter Review
Attachment #8511139 - Attachment is obsolete: true
Attachment #8511164 - Flags: review?(florian)
Attachment #8511164 - Flags: review?(aleth)
Comment on attachment 8511164 [details] [diff] [review]
null.patch from comment 1

Review of attachment 8511164 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/components/src/imConversations.js
@@ +37,5 @@
> +    // Explicitly test for null so that blank messages don't fall back to
> +    // the original. Especially problematic in encryption extensions like OTR.
> +    return this._displayMessage !== null
> +      ? this._displayMessage
> +      : this.prplMessage.originalMessage;

Nit: Please put operators at the ends of lines, thanks!
(Assignee)

Comment 4

5 years ago
Attachment #8511164 - Attachment is obsolete: true
Attachment #8511164 - Flags: review?(florian)
Attachment #8511164 - Flags: review?(aleth)
Attachment #8511205 - Flags: review?(florian)
Attachment #8511205 - Flags: review?(clokep)
Attachment #8511205 - Flags: review?(aleth)
Attachment #8511205 - Flags: review?(clokep)

Updated

5 years ago
Attachment #8511205 - Flags: review?(florian)
Attachment #8511205 - Flags: review?(aleth)
Attachment #8511205 - Flags: review+

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/6b5ab77843f5
Assignee: nobody → arlolra
Status: UNCONFIRMED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.