Bugfix for headline messages.

Couldn't handle messages with no "from" attribute.
Some refactoring to add code that checks if a messages is a headline to the
utils module.
Updated tests. Add sinon so that we can test returned value of spy.

bower.json

@@ -30,7 +30,8 @@
     "skeleton-sass": "~2.0.3",
     "strophejs": "1.2.4",
     "strophejs-plugins": "https://github.com/strophe/strophejs-plugins.git#amd",
-    "bourbon": "~4.2.3"
+    "bourbon": "~4.2.3",
+    "sinon": "^1.17.3"
   },
   "resolutions": {
     "backbone": "1.1.2"

jshintrc

@@ -20,6 +20,7 @@
         "global",
         "it",
         "jasmine",
+        "sinon",
         "module",
         "require",
         "runs",

spec/chatbox.js

@@ -3,13 +3,11 @@
     define([
         "jquery",
         "underscore",
+        "utils",
         "mock",
         "test_utils"
-        ], function ($, _, mock, test_utils) {
-            return factory($, _, mock, test_utils);
-        }
-    );
-} (this, function ($, _, mock, test_utils) {
+        ], factory);
+} (this, function ($, _, utils, mock, test_utils) {
     var $msg = converse_api.env.$msg;
     var Strophe = converse_api.env.Strophe;
     var moment = converse_api.env.moment;
@@ -461,6 +459,7 @@
                 it("is ignored if it's intended for a different resource", function () {
                     // Send a message from a different resource
                     spyOn(converse, 'log');
+                    spyOn(converse.chatboxes, 'getChatBox');
                     var sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
                     var msg = $msg({
                             from: sender_jid,
@@ -471,7 +470,35 @@
                         .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree();
                     converse.chatboxes.onMessage(msg);
                     expect(converse.log).toHaveBeenCalledWith(
-                            "Ignore incoming message intended for a different resource: dummy@localhost/some-other-resource", "info");
+                            "onMessage: Ignoring incoming message intended for a different resource: dummy@localhost/some-other-resource", "info");
+                    expect(converse.chatboxes.getChatBox).not.toHaveBeenCalled();
+                });
+
+                it("is ignored if it's a malformed headline message", function () {
+                    /* Ideally we wouldn't have to filter out headline
+                     * messages, but Prosody gives them the wrong 'type' :(
+                     */
+                    sinon.spy(converse, 'log');
+                    sinon.spy(converse.chatboxes, 'getChatBox');
+                    sinon.spy(utils, 'isHeadlineMessage');
+                    var msg = $msg({
+                            from: 'localhost',
+                            to: converse.bare_jid,
+                            type: 'chat',
+                            id: (new Date()).getTime()
+                        }).c('body').t("This headline message will not be shown").tree();
+                    converse.chatboxes.onMessage(msg);
+                    expect(converse.log.calledWith(
+                        "onMessage: Ignoring incoming headline message sent with type 'chat' from JID: localhost",
+                        "info"
+                    )).toBeTruthy();
+                    expect(utils.isHeadlineMessage.called).toBeTruthy();
+                    expect(utils.isHeadlineMessage.returned(true)).toBeTruthy();
+                    expect(converse.chatboxes.getChatBox.called).toBeFalsy();
+                    // Remove sinon spies
+                    converse.log.restore();
+                    converse.chatboxes.getChatBox.restore();
+                    utils.isHeadlineMessage.restore();
                 });
 
                 it("can be a carbon message, as defined in XEP-0280", function () {

spec/headline.js

@@ -2,13 +2,11 @@
 (function (root, factory) {
     define([
         "jquery",
+        "utils",
         "mock",
         "test_utils"
-        ], function ($, mock, test_utils) {
-            return factory($, mock, test_utils);
-        }
-    );
-} (this, function ($, mock, test_utils) {
+        ], factory);
+} (this, function ($, utils, mock, test_utils) {
     "use strict";
     var $msg = converse_api.env.$msg,
         _ = converse_api.env._;
@@ -30,6 +28,7 @@
              *  </x>
              *  </message>
              */
+            sinon.spy(utils, 'isHeadlineMessage');
             runs(function () {
                 var stanza = $msg({
                         'type': 'headline',
@@ -50,6 +49,9 @@
                         converse.chatboxviews.keys(),
                         'notify.example.com')
                     ).toBeTruthy();
+                expect(utils.isHeadlineMessage.called).toBeTruthy();
+                expect(utils.isHeadlineMessage.returned(true)).toBeTruthy();
+                utils.isHeadlineMessage.restore(); // unwraps
             });
         });
     });

src/converse-core.js

@@ -1308,22 +1308,39 @@
             },
 
             onMessage: function (message) {
-                /* Handler method for all incoming single-user chat "message" stanzas.
+                /* Handler method for all incoming single-user chat "message"
+                 * stanzas.
                  */
                 var $message = $(message),
-                    contact_jid, $forwarded, $delay, from_bare_jid, from_resource, is_me, msgid,
+                    contact_jid, $forwarded, $delay, from_bare_jid,
+                    from_resource, is_me, msgid,
                     chatbox, resource,
                     from_jid = $message.attr('from'),
                     to_jid = $message.attr('to'),
                     to_resource = Strophe.getResourceFromJid(to_jid);
 
                 if (to_resource && to_resource !== converse.resource) {
-                    converse.log('Ignore incoming message intended for a different resource: '+to_jid, 'info');
+                    converse.log(
+                        'onMessage: Ignoring incoming message intended for a different resource: '+to_jid,
+                        'info'
+                    );
                     return true;
-                }
-                if (from_jid === converse.connection.jid) {
-                    // FIXME: Forwarded messages should be sent to specific resources, not broadcasted
-                    converse.log("Ignore incoming message sent from this client's JID: "+from_jid, 'info');
+                } else if (from_jid === converse.connection.jid) {
+                    // FIXME: Forwarded messages should be sent to specific
+                    // resources, not broadcasted
+                    converse.log(
+                        "onMessage: Ignoring incoming message sent from this client's JID: "+from_jid,
+                        'info'
+                    );
+                    return true;
+                } else if (utils.isHeadlineMessage(message)) {
+                    // XXX: Ideally we wouldn't have to check for headline
+                    // messages, but Prosody sends headline messages with the
+                    // wrong type ('chat'), so we need to filter them out here.
+                    converse.log(
+                        "onMessage: Ignoring incoming headline message sent with type 'chat' from JID: "+from_jid,
+                        'info'
+                    );
                     return true;
                 }
                 $forwarded = $message.find('forwarded');

src/converse-headline.js

@@ -22,12 +22,9 @@
     var onHeadlineMessage = function (message) {
         /* Handler method for all incoming messages of type "headline".
          */
-        var $message = $(message), from_jid = $message.attr('from');
-        if ($message.attr('type') === 'headline' || from_jid.indexOf('@') === -1) {
-            // Some servers (I'm looking at you Prosody) don't set the message
-            // type to "headline" when sending server messages. For now we
-            // check if an @ signal is included, and if not, we assume it's
-            // a headline message.
+        var $message = $(message),
+            from_jid = $message.attr('from');
+        if (utils.isHeadlineMessage(message)) {
             converse.chatboxes.create({
                 'id': from_jid,
                 'jid': from_jid,

src/converse-notification.js

@@ -68,19 +68,16 @@
                 return true;
             };
 
-            converse.shouldNotifyOfMessage = function ($message) {
+            converse.shouldNotifyOfMessage = function (message) {
                 /* Is this a message worthy of notification?
                  */
-                var $forwarded = $message.find('forwarded');
+                var $message = $(message),
+                    $forwarded = $message.find('forwarded');
                 if ($forwarded.length) {
                     return false;
-                }
-                if ($message.attr('type') === 'groupchat') {
+                } else if ($message.attr('type') === 'groupchat') {
                     return converse.shouldNotifyOfGroupMessage($message);
-                }
-                if ($message.attr('type') === 'headline' || $message.attr('from').indexOf('@') === -1) {
-                    // XXX: 2nd check is workaround for Prosody which doesn't give type "headline"
-
+                } else if (utils.isHeadlineMessage(message)) {
                     // We want to show notifications for headline messages.
                     return true;
                 }
@@ -198,7 +195,7 @@
                  * to play sounds and show HTML5 notifications.
                  */
                 var $message = $(message);
-                if (!converse.shouldNotifyOfMessage($message)) {
+                if (!converse.shouldNotifyOfMessage(message)) {
                     return false;
                 }
                 converse.playSoundNotification($message);

src/utils.js

@@ -125,6 +125,21 @@
             return str;
         },
 
+        isHeadlineMessage: function (message) {
+            var $message = $(message),
+                from_jid = $message.attr('from');
+            if ($message.attr('type') === 'headline' ||
+                    // Some servers (I'm looking at you Prosody) don't set the message
+                    // type to "headline" when sending server messages. For now we
+                    // check if an @ signal is included, and if not, we assume it's
+                    // a headline message.
+                    (typeof from_jid !== 'undefined' && from_jid.indexOf('@') === -1)
+                ) {
+                return true;
+            }
+            return false;
+        },
+
         refreshWebkit: function () {
             /* This works around a webkit bug. Refreshes the browser's viewport,
              * otherwise chatboxes are not moved along when one is closed.

tests/main.js

@@ -1,6 +1,7 @@
 // Extra test dependencies
 config.paths.mock = "tests/mock";
 config.paths.test_utils = "tests/utils";
+config.paths.sinon = "components/sinon/lib/sinon";
 config.paths.jasmine = "components/jasmine/lib/jasmine-core/jasmine";
 config.paths["jasmine-html"] = "components/jasmine/lib/jasmine-core/jasmine-html";
 config.paths["console-runner"] = "node_modules/phantom-jasmine/lib/console-runner";
@@ -34,9 +35,11 @@ require([
     "jquery",
     "converse",
     "mock",
-    "jasmine-html"
-    ], function($, converse, mock, jasmine) {
+    "jasmine-html",
+    "sinon"
+    ], function($, converse, mock, jasmine, sinon) {
         // Set up converse.js
+        window.sinon = sinon;
         window.converse_api = converse;
         window.localStorage.clear();
         window.sessionStorage.clear();