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

Status: RESOLVED FIXED

(Instantbird Graveyard :: Other, enhancement)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: 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).

Comment 1

[aleth [:aleth]]

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)

Comment 2

[Patrick Cloke [:clokep]]

Any idea if this is still necessary?

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/comm-central/rev/e3a88b8c91e2267ace82651ec29049f1f451aaca