Use array.includes in chat and im code instead of indexOf != / == -1

RESOLVED FIXED in Instantbird 51

Status

Instantbird
Other
--
enhancement
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: aryx, Unassigned)

Tracking

Instantbird 51

Details

Attachments

(1 attachment)

Created attachment 8666472 [details] [diff] [review]
patch, v1

From bug 1207363 comment 2:
No, I haven't written a script for this, I do this for relaxing and looking at code to learn and spot other bugs (found bug 1207364 that way).
Attachment #8666472 - Flags: review?(aleth)

Comment 1

3 years ago
Comment on attachment 8666472 [details] [diff] [review]
patch, v1

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

Thanks! Now you know the chat code ;) 

r+ with the following nits fixed.

::: chat/modules/jsProtoHelper.jsm
@@ +468,5 @@
>      this._id = aId;
>    },
>  
>    addObserver: function(aObserver) {
> +    if (!this._observers.includes(aObserver))

Really all these _observers should be Maps or WeakMaps, but that's for a separate bug...

::: chat/protocols/irc/irc.js
@@ +862,5 @@
>    },
>    normalizeNick: function(aNick) { return this.normalize(aNick, this.userPrefixes); },
>  
>    isMUCName: function(aStr) {
> +    return (this.channelPrefixes.includes(aStr[0]));

Nit: remove the surrounding ()

::: chat/protocols/twitter/twitter-text.jsm
@@ +47,5 @@
>    // Builds a RegExp
>    function regexSupplant(regex, flags) {
>      flags = flags || "";
>      if (typeof regex !== "string") {
> +      if (regex.global && !flags.includes("g")) {

Please undo the changes to this file as it's an imported library.

::: im/content/conversation.xml
@@ +516,5 @@
>                                 KeyEvent.DOM_VK_UP, KeyEvent.DOM_VK_DOWN];
>  
>            // Pass navigation keys to the browser if
>            // 1) the textbox is empty or 2) it's an IB-specific key combination
> +          if ((!text && navKeyCodes.includes(event.keyCode)) ||

Nit: remove extra () that are now not needed

::: im/modules/imWindows.jsm
@@ +184,5 @@
>  
>    showConversation: function(aConv) {
>      if (this.isUIConversationDisplayed(aConv) ||
>          (this._pendingConversations &&
> +        this._pendingConversations.includes(aConv)))

Nit: does this now fit on the previous line? (<=80 chars)
Attachment #8666472 - Flags: review?(aleth) → review+
Any idea if this is still necessary?
Flags: needinfo?(aryx.bugmail)
https://hg.mozilla.org/comm-central/rev/e3a88b8c91e2267ace82651ec29049f1f451aaca
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(aryx.bugmail)
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 51
Version: trunk → 51
You need to log in before you can comment on or make changes to this bug.