Closed
Bug 821719
Opened 12 years ago
Closed 12 years ago
Use newer JavaScript features in compose code
Categories
(Thunderbird :: Message Compose Window, enhancement)
Thunderbird
Message Compose Window
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 821619
People
(Reporter: aryx, Assigned: aryx)
Details
Attachments
(1 file)
(deleted),
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
/mail/components/compose/content:
- use <string>.startsWith/endsWith/contains
- use querySelector instead of getElementsBy<foo>[0]
etc.
Successful Try run: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=61746c45585c
The patch will be applied after bug 821619
Attachment #692301 -
Flags: review?(mconley)
Comment 1•12 years ago
|
||
Comment on attachment 692301 [details] [diff] [review]
code change, patch v1
Review of attachment 692301 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with the following nits fixed. Thanks!
::: mail/components/compose/content/MsgComposeCommands.js
@@ +2264,5 @@
>
> // " <>" is an empty identity, and most likely not valid
> if (!params.identity || params.identity.identityName == " <>") {
> // no pre selected identity, so use the default account
> + var identities = MailServices.accounts.defaultAccount.identities;
Switch var to let.
@@ +2719,5 @@
>
> // Check if the user tries to send a message to a newsgroup through a mail
> // account.
> var currentAccountKey = getCurrentAccountKey();
> + var account = MailServices.accounts.getAccount(currentAccountKey);
Switch var to let
@@ +3083,5 @@
> var emailAddresses = {};
> var names = {};
> var fullNames = {};
> + var numAddresses = MailServices.headerParser
> + .parseHeadersWithArray(aAddressesToAdd, emailAddresses, names, fullNames);
This line is still quite long. Maybe break this up like:
let numAddresses =
MailServices.headerParser.parseHeadersWithArray(aAddressesToAdd,
emailAddresses,
names,
fullNames);
@@ +3090,5 @@
> var tokenizedNames = new Array();
>
> // each name could consist of multiple word delimited by either commas or spaces. i.e. Green Lantern
> // or Lantern,Green. Tokenize on comma first, then tokenize again on spaces.
> + for (var name in names.value)
let, not var
@@ +3313,5 @@
> }
>
> function getCurrentAccountKey()
> {
> // get the accounts key
While you're here, fix the indentation here to two spaces.
@@ +3320,5 @@
> }
>
> function getIdentityForKey(key)
> {
> + return MailServices.accounts.getIdentity(key);
Use two spaces here.
@@ +4249,5 @@
> item.flavour.contentType == "application/x-moz-file")
> {
> if (item.flavour.contentType == "application/x-moz-file")
> {
> + var fileHandler = Services.io
let, not var
::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +1033,4 @@
> var addresses = {};
> var names = {};
> var fullNames = {};
> + var numAddresses = MailServices.headerParser
let numAddresses =
MailServices.headerParser.parseHeadersWithArray(strippedAddresses,
addresses,
names,
fullNames);
Attachment #692301 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 2•12 years ago
|
||
Thank you for the review, seems like attached the same patch like in bug 821619. I merged them into one and dupe this bug against bug 821619.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•