Review of attachment 9200976 [details] [diff] [review]: ----------------------------------------------------------------- Looks pretty good, but I think the rule is also needed for tabs as well as spaces, right? ::: mailnews/compose/src/MimeEncoder.jsm @@ +407,5 @@ > } > > if (currentColumn >= 73) { > // Soft line break for readability > + if (i + 1 < this._bodySize && this._body[i + 1] == " ") { I think [QP](https://tools.ietf.org/html/rfc2045#section-6.7) says this rule needs to apply for tabs as well as spaces? ::: mailnews/compose/test/unit/test_messageBody.js @@ +68,5 @@ > + > + fields = new CompFields(); > + fields.forceMsgEncoding = true; > + fields.to = "Nobody <nobody@tinderbox.invalid>"; > + fields.subject = "Test QP encoding for non-ascii and trailing tab"; "tab"? should be "space"? Actually, I'd ditch the non-ascii part of the string (the "\u00e1", below ) for this test and just concentrate on the trailing space. @@ +76,5 @@ > + msgData = mailTestUtils.loadMessageToString( > + gDraftFolder, > + mailTestUtils.firstMsgHdr(gDraftFolder) > + ); > + let endOfHeaders = msgData.indexOf("\r\n\r\n"); [rfc 2045](https://tools.ietf.org/html/rfc2045#section-6.7) says CRLF is canonical, so I think it's reasonable for the test to expect it. ::: mailnews/mime/src/mimeenc.cpp @@ +965,5 @@ > > if (mCurrentColumn >= 73) // Soft line break for readability > { > *out++ = '='; > + if (in + 1 < end && in[1] == ' ') { Again, tabs too?
Bug 1689804 Comment 44 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Review of attachment 9200976 [details] [diff] [review]: ----------------------------------------------------------------- Looks pretty good, but I think the rule is also needed for tabs as well as spaces, right? ::: mailnews/compose/src/MimeEncoder.jsm @@ +407,5 @@ > } > > if (currentColumn >= 73) { > // Soft line break for readability > + if (i + 1 < this._bodySize && this._body[i + 1] == " ") { I think [QP](https://tools.ietf.org/html/rfc2045#section-6.7) says this rule needs to apply for tabs as well as spaces? ::: mailnews/compose/test/unit/test_messageBody.js @@ +68,5 @@ > + > + fields = new CompFields(); > + fields.forceMsgEncoding = true; > + fields.to = "Nobody <nobody@tinderbox.invalid>"; > + fields.subject = "Test QP encoding for non-ascii and trailing tab"; "tab"? should be "space"? Actually, I'd ditch the non-ascii part of the string (the "\u00e1", below ) for this test and just concentrate on the trailing space. @@ +76,5 @@ > + msgData = mailTestUtils.loadMessageToString( > + gDraftFolder, > + mailTestUtils.firstMsgHdr(gDraftFolder) > + ); > + let endOfHeaders = msgData.indexOf("\r\n\r\n"); (EDIT: not clear here, but this comment was replying to the question from Klaus about platform-dependent line endings) [rfc 2045](https://tools.ietf.org/html/rfc2045#section-6.7) says CRLF is canonical, so I think it's reasonable for the test to expect it. ::: mailnews/mime/src/mimeenc.cpp @@ +965,5 @@ > > if (mCurrentColumn >= 73) // Soft line break for readability > { > *out++ = '='; > + if (in + 1 < end && in[1] == ' ') { Again, tabs too?