1298696 - Apply JS code optimizations from SeaMonkey bug 1286703 to Thunderbird bug 903390

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[rsx11m]

(Quoting Philip Chee from bug 1286703 comment #2)
> Comment on attachment 8771573 [details] [diff] [review]
> Proposed fix
> 
> r=me a=me
> 
> > +      let identityList = document.getElementById("msgIdentity");
> let identityList = GetMsgIdentityElement();
> 
> >        if (currentNode.nodeName == "BR") {
> >          currentNode.remove();
> > +        currentNode = mailBody.childNodes[start];
> > +        if (currentNode && currentNode.nodeName == "BR") {
> > +          mailBody.removeChild(currentNode);
> 
> currentNode.remove() just like a few lines above.
> 
> > +          let bccAddrs = new Set(msgCompFields.splitRecipients(msgCompFields.bcc, true, {}));
> This is used where?
> 
> > -            if (newCc != "")
> > +            if (newCc != "") {
> newCC is a string so you can simplify with if(newCC)
> 
> > +              // Ensure none of the Ccs are already in To.
> > +              let cc2 = msgCompFields.splitRecipients(newCc, true, {});
> > +              for (let i = cc2.length - 1; i >= 0; i--) {
> > +                if (toAddrs.has(cc2[i])) {
> > +                  cc2.splice(i, 1);
> > +                }
> JavaScript arrays have this nifty built in method Array.filter().
> 
> > +              }
> > +              newCc = cc2.join(", ");
> 
> newCC = cc2.filter(x => !toAddrs.has(x)).join(", ");
> 
> ... or more verbosely:
> 
> cc2 = cc2.filter(x => !toAddrs.has(x));
> newCC = cc2.join(", ");
> 
> > +            if (newBcc != "") {
> ditto
> 
> > +              // Ensure none of the Bccs are already in To or Cc.
> > +              let bcc2 = msgCompFields.splitRecipients(newBcc, true, {});
> > +              for (let i = bcc2.length - 1; i >= 0; i--) {
> > +                if (toAddrs.has(bcc2[i]) || ccAddrs.has(bcc2[i])) {
> > +                  bcc2.splice(i, 1);
> > +                }
> > +              }
> > +              newBcc = bcc2.join(", ");
> 
> let toCcAddrs = new Set([...toAddrs, ...ccAddrs]); // ES6 ...spread operator
> newBcc = bcc2.filter(x => !toCcAddrs.has(x)).join(", ");

(Quoting Jorg K (GMT+2, PTO during summer) from bug 1286703 comment #3)
> Ratty always comes late with his brilliant ideas ;-((
> Rsx11m, would you be able to submit a patch for TB as well that includes
> this clean-up. Thanking you in advance already ;-)

Comment 1

[rsx11m]

Attachment: Port from attachment 8786138

This patch corresponds to https://bugzilla.mozilla.org/attachment.cgi?oldid=8771573&action=interdiff&newid=8786138&headers=1 from bug 1286703 for SeaMonkey.

Works without any user-visible changes in behavior, composition tests are passed.

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 8786140 [details] [diff] [review]
Port from attachment 8786138 [details] [diff] [review]

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

(In reply to rsx11m from comment #1)
> Works without any user-visible changes in behavior, composition tests are
> passed.
So the Ratty magic works, right? ;-)
I haven't tried it but I take your word for it.

Comment 3

[rsx11m]

Yeah, I've tried various combinations of To/Cc/Bcc entries and the "right" ones were removed.
The syntax of those constructs is weird but they seem to work as advertised.

Pushed as http://hg.mozilla.org/comm-central/rev/8c7ddccf95c4

Comment 4

[Jorg K (CEST = GMT+2)]

Comment on attachment 8786140 [details] [diff] [review]
Port from attachment 8786138 [details] [diff] [review]

[Approval Request Comment]
Regression caused by (bug #): Bug 903390
User impact if declined: None.
Testing completed (on c-c, etc.): Manual.
Risk to taking this patch (and alternatives if risky):
Low, the idea is to let the JS code optimizations join the original landing.
(Rsx11m - I'll land myself ;-))

Comment 5

[Jorg K (CEST = GMT+2)]

Aurora (TB 50):
https://hg.mozilla.org/releases/comm-aurora/rev/459d9584f945