Closed Bug 1392529 Opened 3 years ago Closed 3 years ago

Port |Bug 1385172 - Replace nsEscapeHTML{,2}() with new nsAppendEscapedHTML() function| to mailnews

Categories

(MailNews Core :: Backend, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 57.0

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

Details

Attachments

(3 files)

We have
mailnews/base/util/nsMsgUtils.h
#define MsgEscapeHTML(str) nsEscapeHTML(str)
#define MsgEscapeHTML2(buffer, len) nsEscapeHTML2(buffer, len)
and use MsgEscapeHTML quite a bit.
Attachment #8899762 - Flags: review?(acelists)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/80e43346d487
Port bug 1385172 to mailnews: Replace nsEscapeHTML{,2}() with new nsAppendEscapedHTML() function. rs=bustage-fix
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Assignee: nobody → jorgk
Target Milestone: --- → Thunderbird 57.0
Comment on attachment 8899762 [details] [diff] [review]
1392529-nsEscapeHTML.patch

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

::: mailnews/mime/src/mimeebod.cpp
@@ -396,4 @@
>            goto FAIL;
>          }
>          PL_strcpy(body, pre);
> -        PL_strcat(body, s2);

Note that s2 leaked :-(
Follow-up as suggested by Eric Rahm in bug 1385172 comment #15.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/6136a4dd0f4d
Follow-up: use ToNewCString() instead of strdup(). r=me DONTBUILD
Comment on attachment 8899762 [details] [diff] [review]
1392529-nsEscapeHTML.patch

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

Thanks.

::: mailnews/mime/emitters/nsMimeBaseEmitter.cpp
@@ +878,1 @@
>        mHTMLHeaders.Append(escapedName);

Would it work to directly append to mHTMLHeaders?
nsAppendEscapedHTML(name, mHTMLHeaders);
Attachment #8899762 - Flags: review?(acelists) → review+
Attachment #8899935 - Flags: review+
(In reply to :aceman from comment #6)
> Would it work to directly append to mHTMLHeaders?
> nsAppendEscapedHTML(name, mHTMLHeaders);
Yep. I'll do another follow-up.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/be1c2f371496
Follow-up (take 2): Simplification. r=aceman DONTBUILD
You need to log in before you can comment on or make changes to this bug.