Bug 1206238 - Improve XMPP handleErrors to display error system messages directly. r=clokep
authoraleth <aleth@instantbird.org>
Wed, 23 Sep 2015 01:53:34 +0200
changeset 18418 a5679881ec4c40a1e2441ee27cb5d07db70c9a5e
parent 18417 e326fb651da9c16bfd64f77c19b72c92d1d4c214
child 18419 8ffef885f4b502eaedf95e260237f59c1b9ed3d6
push id11279
push useraleth@instantbird.org
push dateWed, 23 Sep 2015 00:08:16 +0000
treeherdercomm-central@5f1a420df1af [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersclokep
bugs1206238
Bug 1206238 - Improve XMPP handleErrors to display error system messages directly. r=clokep
chat/protocols/xmpp/xmpp.jsm
--- a/chat/protocols/xmpp/xmpp.jsm
+++ b/chat/protocols/xmpp/xmpp.jsm
@@ -165,49 +165,38 @@ const XMPPMUCConversationPrototype = {
 
   _targetResource: "",
 
   // True while we are rejoining a room previously parted by the user.
   _rejoined: false,
 
   get topic() this._topic,
   set topic(aTopic) {
-    let notAuthorized = (aError) => {
-      // XEP-0045 (8.1): Unauthorized subject change.
-      let message = _("conversation.error.changeTopicFailedNotAuthorized");
-      this.writeMessage(this.name, message, {system: true, error: true});
-      return true;
-    };
-    let errorHandler = this._account.handleErrors({forbidden: notAuthorized,
-                                                   notAcceptable: notAuthorized,
-                                                   itemNotFound: notAuthorized});
-
     // XEP-0045 (8.1): Modifying the room subject.
     let subject = Stanza.node("subject", null, null, aTopic.trim());
     let s = Stanza.message(this.name, null, null,{type: "groupchat"}, subject);
-    this._account.sendStanza(s, errorHandler);
+    let notAuthorized = _("conversation.error.changeTopicFailedNotAuthorized");
+    this._account.sendStanza(s, this._account.handleErrors({
+      forbidden: notAuthorized,
+      notAcceptable: notAuthorized,
+      itemNotFound: notAuthorized
+    }, this));
   },
   get topicSettable() true,
 
   /* Called when the user enters a chat message */
   sendMsg: function (aMsg) {
-    let notInRoom = (aError) => {
-      // If the sender is not in the room.
-      let from = aError.stanza.attributes["from"];
-      let message = _("conversation.error.sendFailedAsNotInRoom",
-                      this._account.normalize(from), aMsg);
-      this.writeMessage(from, message, {system: true, error: true});
-      return true;
-    };
-    let errorHandler = this._account.handleErrors({itemNotFound: notInRoom,
-                                                   notAcceptable: notInRoom});
-
     // XEP-0045 (7.4): Sending a message to all occupants in a room.
     let s = Stanza.message(this.name, aMsg, null, {type: "groupchat"});
-    this._account.sendStanza(s, errorHandler);
+    let notInRoom = _("conversation.error.sendFailedAsNotInRoom",
+                      this.name, aMsg);
+    this._account.sendStanza(s, this._account.handleErrors({
+      itemNotFound: notInRoom,
+      notAcceptable: notInRoom
+    }, this));
   },
 
   /* Called by the account when a presence stanza is received for this muc */
   onPresenceStanza: function(aStanza) {
     let from = aStanza.attributes["from"];
     let nick = this._account._parseJID(from).resource;
     let jid = this._account.normalize(from);
     let x = aStanza.getElements(["x"]).find(e => e.uri == Stanza.NS.muc_user);
@@ -467,61 +456,38 @@ const XMPPMUCConversationPrototype = {
       aMsg ? Stanza.node("reason", null, null, aMsg) : null);
     let s = Stanza.iq("set", null, this.name,
                       Stanza.node("query", Stanza.NS.muc_admin, null, item));
     this._account.sendStanza(s, this._banKickHandler, this);
   },
 
   // Callback for ban and kick commands.
   _banKickHandler: function(aStanza) {
-    if (aStanza.attributes["type"] == "error") {
-      let error = this._account.parseError(aStanza);
-      let message;
-      switch (error.condition) {
-        case "not-allowed":
-          message = _("conversation.error.banKickCommandNotAllowed");
-          break;
-        case "conflict":
-          message = _("conversation.error.banKickCommandConflict");
-          break;
-        default:
-          return false;
-      }
-      this.writeMessage(this.name, message, {system: true});
+    if (aStanza.attributes["type"] == "result")
       return true;
-    }
-    else if (aStanza.attributes["type"] == "result")
-      return true;
-    return false;
+    let errorHandler = this._account.handleErrors({
+      notAllowed: _("conversation.error.banKickCommandNotAllowed"),
+      conflict: _("conversation.error.banKickCommandConflict")
+    }, this);
+    return errorHandler(aStanza);
   },
 
   // Changes nick in MUC conversation to a new one.
   setNick: function(aNewNick) {
-    let notAcceptable = (aError) => {
-      // XEP-0045 (7.6): Changing Nickname (example 53).
-      let message = _("conversation.error.changeNickFailedNotAcceptable",
-                      aNewNick);
-      this.writeMessage(this.name, message, {system: true, error: true});
-      // TODO: We should then discover user's reserved nickname (it could be
-      // discovered before joining a room).
-      // XEP-0045 (7.12): Discovering Reserved Room Nickname.
-      return true;
-    };
-    let conflict = (aError) => {
-      // XEP-0045 (7.2.9): Nickname Conflict.
-      let message = _("conversation.error.changeNickFailedConflict", aNewNick);
-      this.writeMessage(this.name, message, {system: true, error: true});
-      return true;
-    };
-    let errorHandler = this._account.handleErrors({notAcceptable: notAcceptable,
-                                                   conflict: conflict});
-
     // XEP-0045 (7.6): Changing Nickname.
     let s = Stanza.presence({to: this.name + "/" + aNewNick}, null);
-    this._account.sendStanza(s, errorHandler);
+    this._account.sendStanza(s, this._account.handleErrors({
+      // XEP-0045 (7.6): Changing Nickname (example 53).
+      // TODO: We should discover if the user has a reserved nickname (maybe
+      // before joining a room), cf. XEP-0045 (7.12).
+      notAcceptable: _("conversation.error.changeNickFailedNotAcceptable",
+                       aNewNick),
+      // XEP-0045 (7.2.9): Nickname Conflict.
+      conflict: _("conversation.error.changeNickFailedConflict", aNewNick)
+    }, this));
   },
 
   /* Called when the user closed the conversation */
   close: function() {
     if (!this.left)
       this.part();
     GenericConvChatPrototype.close.call(this);
   },
@@ -1406,37 +1372,64 @@ const XMPPAccountPrototype = {
     let errortext = error.getElement(["text"]);
     if (errortext)
       retval.text = errortext.innerText;
 
     return retval;
   },
 
   // Returns an error-handling callback for use with sendStanza generated
-  // from aHandlers, an object containing the error handlers.
-  // If the stanza passed to the callback is an error stanza, and
-  // aHandlers contains a method with the name of the defined condition
-  // of the error, that method is called. It should return true if the
-  // error was handled.
+  // from aHandlers, an object defining the error handlers.
+  // If the stanza passed to the callback is an error stanza, it checks if
+  // aHandlers contains a property with the name of the defined condition
+  // of the error.
+  // * If the property is a function, it is called with the parsed error
+  //   as its argument, bound to aThis (if provided).
+  //   It should return true if the error was handled.
+  // * If the property is a string, it is displayed as a system message
+  //   in the conversation given by aThis.
   handleErrors(aHandlers, aThis) {
     return (aStanza) => {
       let error = this.parseError(aStanza);
       if (!error)
         return false;
+
       let toCamelCase = aStr => {
         // Turn defined condition string into a valid camelcase
         // JS property name.
         let capitalize = s => (s[0].toUpperCase() + s.slice(1));
         let uncapitalize = s => (s[0].toLowerCase() + s.slice(1));
         return uncapitalize(aStr.split("-").map(capitalize).join(""));
       }
       let condition = toCamelCase(error.condition);
+      // Check if we have a handler property for this kind of error.
       if (!(condition in aHandlers))
         return false;
-      return aHandlers[condition].call(aThis, error);
+
+      let handler = aHandlers[condition];
+      if (typeof handler == "string") {
+        // The string is an error message to be displayed in the conversation.
+        if (!aThis || !aThis.writeMessage) {
+          this.ERROR("HandleErrors was passed an error message string, but " +
+            "no conversation to display it in:\n" + handler);
+          return true;
+        }
+        aThis.writeMessage(aThis.name, handler, {system: true, error: true});
+        return true;
+      }
+      else if (typeof handler == "function") {
+        // If we're given a function, call this error handler.
+        return handler.call(aThis, error);
+      }
+      else {
+        // If this happens, there's a bug somewhere.
+        this.ERROR("HandleErrors was passed a handler for '" + condition +
+          "'' which is neither a function nor a string.");
+        return false;
+      }
     };
   },
 
   // Send an error stanza in response to the given stanza (rfc6120#8.3).
   // aCondition is the name of the defined-condition child, aText an
   // optional plain-text description.
   sendErrorStanza(aStanza, aCondition, aType, aText) {
     // TODO: Support the other stanza types (message, presence).