From 62c170273e8292df517243583ac9425b4655ada2 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Thu, 9 Jun 2016 11:01:54 +0200 Subject: [PATCH 1/7] Allow plugins to have optional dependencies. This change refactors out the plugin code from converse-core into src/converse-puggable.js Additionally, plugins now have an optional attribute `optional_dependencies` which is an array of dependencies which are "nice-to-have" but not essential. Work has also been done to ensure that a plugins' dependencies are first loaded before the plugin itself. --- converse.js | 3 +- dev.html | 1 + spec/chatbox.js | 12 +- spec/converse.js | 2 +- spec/protocol.js | 9 +- src/converse-api.js | 11 +- src/converse-chatview.js | 2 +- src/converse-controlbox.js | 2 +- src/converse-core.js | 132 ++++------------------ src/converse-dragresize.js | 2 +- src/converse-headline.js | 2 +- src/converse-mam.js | 2 +- src/converse-minimize.js | 2 +- src/converse-muc.js | 15 ++- src/converse-notification.js | 2 +- src/converse-otr.js | 2 +- src/converse-ping.js | 2 +- src/converse-pluggable.js | 208 +++++++++++++++++++++++++++++++++++ src/converse-register.js | 2 +- src/converse-vcard.js | 2 +- src/utils.js | 1 - 21 files changed, 281 insertions(+), 135 deletions(-) create mode 100644 src/converse-pluggable.js diff --git a/converse.js b/converse.js index 69f79153b..9c5a71b3b 100755 --- a/converse.js +++ b/converse.js @@ -56,6 +56,7 @@ require.config({ "converse-notification": "src/converse-notification", "converse-otr": "src/converse-otr", "converse-ping": "src/converse-ping", + "converse-pluggable": "src/converse-pluggable", "converse-register": "src/converse-register", "converse-rosterview": "src/converse-rosterview", "converse-templates": "src/converse-templates", @@ -235,11 +236,11 @@ if (typeof define !== 'undefined') { // translations that you care about. "converse-chatview", // Renders standalone chat boxes for single user chat + "converse-controlbox", // The control box "converse-mam", // XEP-0313 Message Archive Management "converse-muc", // XEP-0045 Multi-user chat "converse-vcard", // XEP-0054 VCard-temp "converse-otr", // Off-the-record encryption for one-on-one messages - "converse-controlbox", // The control box "converse-register", // XEP-0077 In-band registration "converse-ping", // XEP-0199 XMPP Ping "converse-notification",// HTML5 Notifications diff --git a/dev.html b/dev.html index d007d9a83..785eb0674 100644 --- a/dev.html +++ b/dev.html @@ -68,6 +68,7 @@ play_sounds: true, roster_groups: true, show_controlbox_by_default: true, + strict_plugin_dependencies: false, chatstate_notification_blacklist: ['mulles@movim.eu'], xhr_user_search: false, debug: true diff --git a/spec/chatbox.js b/spec/chatbox.js index 84a3c585e..08b6768c0 100644 --- a/spec/chatbox.js +++ b/spec/chatbox.js @@ -406,11 +406,13 @@ describe("A Chat Message", function () { beforeEach(function () { - runs(function () { - test_utils.closeAllChatBoxes(); - }); - waits(250); - runs(function () {}); + test_utils.closeAllChatBoxes(); + test_utils.removeControlBox(); + converse.roster.browserStorage._clear(); + test_utils.initConverse(); + test_utils.createContacts('current'); + test_utils.openControlBox(); + test_utils.openContactsPanel(); }); describe("when received from someone else", function () { diff --git a/spec/converse.js b/spec/converse.js index 17076ef2b..843ec33d9 100644 --- a/spec/converse.js +++ b/spec/converse.js @@ -20,7 +20,7 @@ var connection = converse.connection; delete converse.bosh_service_url; delete converse.connection; - expect(converse.initConnection.bind({})).toThrow( + expect(converse.initConnection.bind(converse)).toThrow( new Error("initConnection: you must supply a value for either the bosh_service_url or websocket_url or both.")); converse.bosh_service_url = url; converse.connection = connection; diff --git a/spec/protocol.js b/spec/protocol.js index 30adfc068..6313dbbd0 100644 --- a/spec/protocol.js +++ b/spec/protocol.js @@ -17,6 +17,12 @@ // https://xmpp.org/rfcs/rfc3921.html describe("The Protocol", $.proxy(function (mock, test_utils) { + beforeEach(function () { + test_utils.removeControlBox(); + converse.roster.browserStorage._clear(); + test_utils.initConverse(); + }); + describe("Integration of Roster Items and Presence Subscriptions", $.proxy(function (mock, test_utils) { /* Some level of integration between roster items and presence * subscriptions is normally expected by an instant messaging user @@ -48,9 +54,6 @@ */ beforeEach(function () { test_utils.closeAllChatBoxes(); - test_utils.removeControlBox(); - converse.roster.browserStorage._clear(); - test_utils.initConverse(); test_utils.openControlBox(); test_utils.openContactsPanel(); }); diff --git a/src/converse-api.js b/src/converse-api.js index 0243232f7..b08ff1638 100644 --- a/src/converse-api.js +++ b/src/converse-api.js @@ -133,7 +133,13 @@ } else if (typeof jids === "string") { return converse.wrappedChatBox(converse.chatboxes.getChatBox(jids, true)); } - return _.map(jids, _.partial(_.compose(converse.wrappedChatBox, converse.chatboxes.getChatBox.bind(converse.chatboxes)), _, true)); + return _.map(jids, + _.partial( + _.compose( + converse.wrappedChatBox.bind(converse), converse.chatboxes.getChatBox.bind(converse.chatboxes) + ), _, true + ) + ); } }, 'tokens': { @@ -181,7 +187,8 @@ }, 'plugins': { 'add': function (name, plugin) { - converse.plugins[name] = plugin; + plugin.__name__ = name; + converse.pluggable.plugins[name] = plugin; }, 'remove': function (name) { delete converse.plugins[name]; diff --git a/src/converse-chatview.js b/src/converse-chatview.js index 60f809536..070f4af47 100644 --- a/src/converse-chatview.js +++ b/src/converse-chatview.js @@ -24,7 +24,7 @@ }; - converse_api.plugins.add('chatview', { + converse_api.plugins.add('converse-chatview', { overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/converse-controlbox.js b/src/converse-controlbox.js index 44b8015c5..519ada25d 100644 --- a/src/converse-controlbox.js +++ b/src/converse-controlbox.js @@ -27,7 +27,7 @@ moment = converse_api.env.moment; - converse_api.plugins.add('controlbox', { + converse_api.plugins.add('converse-controlbox', { overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/converse-core.js b/src/converse-core.js index 806b621cd..594449104 100755 --- a/src/converse-core.js +++ b/src/converse-core.js @@ -23,11 +23,12 @@ "moment_with_locales", "strophe", "converse-templates", + "converse-pluggable", "strophe.disco", "backbone.browserStorage", "backbone.overview", ], factory); -}(this, function ($, _, dummy, utils, moment, Strophe, templates) { +}(this, function ($, _, dummy, utils, moment, Strophe, templates, pluggable) { /* * Cannot use this due to Safari bug. * See https://github.com/jcbrand/converse.js/issues/196 @@ -58,26 +59,31 @@ var event_context = {}; var converse = { - plugins: {}, - initialized_plugins: [], templates: templates, + emit: function (evt, data) { $(event_context).trigger(evt, data); }, + once: function (evt, handler) { $(event_context).one(evt, handler); }, + on: function (evt, handler) { if (_.contains(['ready', 'initialized'], evt)) { converse.log('Warning: The "'+evt+'" event has been deprecated and will be removed, please use "connected".'); } $(event_context).bind(evt, handler); }, + off: function (evt, handler) { $(event_context).unbind(evt, handler); } }; + // Make converse pluggable + pluggable.enable(converse); + // Module-level constants converse.STATUS_WEIGHTS = { 'offline': 6, @@ -1770,117 +1776,27 @@ return this; }; - this.wrappedOverride = function (key, value, super_method) { - // We create a partially applied wrapper function, that - // makes sure to set the proper super method when the - // overriding method is called. This is done to enable - // chaining of plugin methods, all the way up to the - // original method. - this._super[key] = super_method; - return value.apply(this, _.rest(arguments, 3)); - }; - - this._overrideAttribute = function (key, plugin) { - // See converse.plugins.override - var value = plugin.overrides[key]; - if (typeof value === "function") { - var wrapped_function = _.partial( - converse.wrappedOverride.bind(converse), - key, value, converse[key].bind(converse) - ); - converse[key] = wrapped_function; - } else { - converse[key] = value; - } - }; - - this._extendObject = function (obj, attributes) { - // See converse.plugins.extend - if (!obj.prototype._super) { - obj.prototype._super = {'converse': converse}; - } - _.each(attributes, function (value, key) { - if (key === 'events') { - obj.prototype[key] = _.extend(value, obj.prototype[key]); - } else if (typeof value === 'function') { - // We create a partially applied wrapper function, that - // makes sure to set the proper super method when the - // overriding method is called. This is done to enable - // chaining of plugin methods, all the way up to the - // original method. - var wrapped_function = _.partial( - converse.wrappedOverride, - key, value, obj.prototype[key] - ); - obj.prototype[key] = wrapped_function; - } else { - obj.prototype[key] = value; - } - }); - }; - - this.initializePlugins = function () { - if (typeof converse._super === 'undefined') { - converse._super = { 'converse': converse }; - } - - var updateSettings = function (settings) { - /* Helper method which gets put on the plugin and allows it to - * add more user-facing config settings to converse.js. - */ - _.extend(converse.default_settings, settings); - _.extend(converse, settings); - _.extend(converse, _.pick(converse.user_settings, Object.keys(settings))); - }; - - _.each(_.keys(this.plugins), function (name) { - var plugin = this.plugins[name]; - plugin.updateSettings = updateSettings; - - if (_.contains(this.initialized_plugins, name)) { - // Don't initialize plugins twice, otherwise we get - // infinite recursion in overridden methods. - return; - } - plugin.converse = converse; - _.each(Object.keys(plugin.overrides || {}), function (key) { - /* We automatically override all methods and Backbone views and - * models that are in the "overrides" namespace. - */ - var msg, - override = plugin.overrides[key]; - if (typeof override === "object") { - if (typeof converse[key] === 'undefined') { - msg = "Error: Plugin tried to override "+key+" but it's not found."; - if (converse.strict_plugin_dependencies) { - throw msg; - } else { - converse.log(msg); - return; - } - } - this._extendObject(converse[key], override); - } else { - this._overrideAttribute(key, plugin); - } - }.bind(this)); - - if (typeof plugin.initialize === "function") { - plugin.initialize.bind(plugin)(this); - } - this.initialized_plugins.push(name); - }.bind(this)); - }; - // Initialization // -------------- // This is the end of the initialize method. if (settings.connection) { this.connection = settings.connection; } - this.initializePlugins(); - this._initialize(); - this.registerGlobalEventHandlers(); + var updateSettings = function (settings) { + /* Helper method which gets put on the plugin and allows it to + * add more user-facing config settings to converse.js. + */ + _.extend(converse.default_settings, settings); + _.extend(converse, settings); + _.extend(converse, _.pick(converse.user_settings, Object.keys(settings))); + }; + converse.pluggable.initializePlugins({ + 'updateSettings': updateSettings, + 'converse': converse + }).then(function () { + converse._initialize(); + converse.registerGlobalEventHandlers(); + }); }; return converse; })); diff --git a/src/converse-dragresize.js b/src/converse-dragresize.js index 082c64d5d..23d207c34 100644 --- a/src/converse-dragresize.js +++ b/src/converse-dragresize.js @@ -19,7 +19,7 @@ var $ = converse_api.env.jQuery, _ = converse_api.env._; - converse_api.plugins.add('dragresize', { + converse_api.plugins.add('converse-dragresize', { overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/converse-headline.js b/src/converse-headline.js index 0ada7a488..9b361b728 100644 --- a/src/converse-headline.js +++ b/src/converse-headline.js @@ -36,7 +36,7 @@ return true; }; - converse_api.plugins.add('headline', { + converse_api.plugins.add('converse-headline', { overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/converse-mam.js b/src/converse-mam.js index 44d1137c4..640904ca7 100644 --- a/src/converse-mam.js +++ b/src/converse-mam.js @@ -32,7 +32,7 @@ Strophe.addNamespace('RSM', 'http://jabber.org/protocol/rsm'); - converse_api.plugins.add('mam', { + converse_api.plugins.add('converse-mam', { overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/converse-minimize.js b/src/converse-minimize.js index 830f2fd2b..45e358a0f 100644 --- a/src/converse-minimize.js +++ b/src/converse-minimize.js @@ -23,7 +23,7 @@ utils = converse_api.env.utils, __ = utils.__.bind(converse); - converse_api.plugins.add('minimize', { + converse_api.plugins.add('converse-minimize', { overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/converse-muc.js b/src/converse-muc.js index f10d75345..adc05e22f 100755 --- a/src/converse-muc.js +++ b/src/converse-muc.js @@ -14,8 +14,7 @@ "converse-core", "converse-api", "typeahead", - "converse-chatview", - "converse-controlbox" + "converse-chatview" ], factory); }(this, function (converse, converse_api) { "use strict"; @@ -43,7 +42,17 @@ Strophe.addNamespace('MUC_ROOMCONF', Strophe.NS.MUC + "#roomconfig"); Strophe.addNamespace('MUC_USER', Strophe.NS.MUC + "#user"); - converse_api.plugins.add('muc', { + converse_api.plugins.add('converse-muc', { + /* Optional dependencies are require.js dependencies which might be + * overridden or relied upon if they exist, but otherwise ignored. + * + * However, if the setting "strict_plugin_dependencies" is set to true, + * then these dependencies will be considered required. + * + * Optional dependencies will be available in the initialize method as + * a the "optional_dependencies" attribute of the plugin. + */ + optional_dependencies: ["converse-controlbox"], overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/converse-notification.js b/src/converse-notification.js index 2beb136d1..d671dd879 100644 --- a/src/converse-notification.js +++ b/src/converse-notification.js @@ -21,7 +21,7 @@ var supports_html5_notification = "Notification" in window; - converse_api.plugins.add('notification', { + converse_api.plugins.add('converse-notification', { initialize: function () { /* The initialize function gets called as soon as the plugin is diff --git a/src/converse-otr.js b/src/converse-otr.js index 9c251ff81..3c207955f 100644 --- a/src/converse-otr.js +++ b/src/converse-otr.js @@ -50,7 +50,7 @@ OTR_CLASS_MAPPING[VERIFIED] = 'verified'; OTR_CLASS_MAPPING[FINISHED] = 'finished'; - converse_api.plugins.add('otr', { + converse_api.plugins.add('converse-otr', { overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/converse-ping.js b/src/converse-ping.js index 1c56a8777..199705e3d 100644 --- a/src/converse-ping.js +++ b/src/converse-ping.js @@ -22,7 +22,7 @@ // Other necessary globals var _ = converse_api.env._; - converse_api.plugins.add('ping', { + converse_api.plugins.add('converse-ping', { initialize: function () { /* The initialize function gets called as soon as the plugin is diff --git a/src/converse-pluggable.js b/src/converse-pluggable.js new file mode 100644 index 000000000..41f273815 --- /dev/null +++ b/src/converse-pluggable.js @@ -0,0 +1,208 @@ +/* + * ____ __ __ __ _ + * / __ \/ /_ __ ___ ___ ____ _/ /_ / /__ (_)____ + * / /_/ / / / / / __ \/ __ \/ __/ / __ \/ / _ \ / / ___/ + * / ____/ / /_/ / /_/ / /_/ / /_/ / /_/ / / __/ / (__ ) + * /_/ /_/\__,_/\__, /\__, /\__/_/_.___/_/\___(_)_/ /____/ + * /____//____/ /___/ + * + */ +(function (root, factory) { + define("converse-pluggable", ["jquery", "underscore"], factory); +}(this, function ($, _) { + "use strict"; + + function Pluggable (plugged) { + this.plugged = plugged; + this.plugged._super = {}; + this.plugins = {}; + this.initialized_plugins = []; + } + _.extend(Pluggable.prototype, { + wrappedOverride: function (key, value, super_method) { + /* We create a partially applied wrapper function, that + * makes sure to set the proper super method when the + * overriding method is called. This is done to enable + * chaining of plugin methods, all the way up to the + * original method. + */ + if (typeof super_method === "function") { + this._super[key] = super_method.bind(this); + } + return value.apply(this, _.rest(arguments, 3)); + }, + + _overrideAttribute: function (key, plugin) { + /* Overrides an attribute on the original object (the thing being + * plugged into). + * + * If the attribute being overridden is a function, then the original + * function will still be available via the _super attribute. + * + * If the same function is being overridden multiple times, then + * the original function will be available at the end of a chain of + * functions, starting from the most recent override, all the way + * back to the original function, each being referenced by the + * previous' _super attribute. + * + * For example: + * + * plugin2.MyFunc._super.myFunc => * plugin1.MyFunc._super.myFunc => original.myFunc + */ + var value = plugin.overrides[key]; + if (typeof value === "function") { + var wrapped_function = _.partial( + this.wrappedOverride, key, value, this.plugged[key] + ); + this.plugged[key] = wrapped_function; + } else { + this.plugged[key] = value; + } + }, + + _extendObject: function (obj, attributes) { + if (!obj.prototype._super) { + // FIXME: make generic + obj.prototype._super = {'converse': this.plugged }; + } + _.each(attributes, function (value, key) { + if (key === 'events') { + obj.prototype[key] = _.extend(value, obj.prototype[key]); + } else if (typeof value === 'function') { + // We create a partially applied wrapper function, that + // makes sure to set the proper super method when the + // overriding method is called. This is done to enable + // chaining of plugin methods, all the way up to the + // original method. + var wrapped_function = _.partial( + this.wrappedOverride, key, value, obj.prototype[key] + ); + obj.prototype[key] = wrapped_function; + } else { + obj.prototype[key] = value; + } + }.bind(this)); + }, + + setOptionalDependencies: function (plugin, dependencies) { + plugin.optional_dependencies = dependencies; + return plugin; + }, + + loadOptionalDependencies: function (plugins) { + var deferred = new $.Deferred(); + require(plugins, + function () { + _.each(plugins, function (name) { + var plugin = this.plugins[name]; + if (plugin) { + this.initializePlugin(plugin).then( + deferred.resolve.bind(this, plugins) + ); + } + }.bind(this)); + }.bind(this), + function () { + if (this.plugged.strict_plugin_dependencies) { + deferred.fail.apply(this, arguments); + this.throwUndefinedDependencyError(arguments[0]); + } else { + deferred.resolve.apply(this, [plugins]); + } + }.bind(this)); + return deferred.promise(); + }, + + throwUndefinedDependencyError: function (msg) { + if (this.plugged.strict_plugin_dependencies) { + throw msg; + } else { + console.log(msg); + return; + } + }, + + applyOverrides: function (plugin) { + _.each(Object.keys(plugin.overrides || {}), function (key) { + /* We automatically override all methods and Backbone views and + * models that are in the "overrides" namespace. + */ + var override = plugin.overrides[key]; + if (typeof override === "object") { + if (typeof this.plugged[key] === 'undefined') { + this.throwUndefinedDependencyError("Error: Plugin \""+plugin.__name__+"\" tried to override "+key+" but it's not found."); + } else { + this._extendObject(this.plugged[key], override); + } + } else { + this._overrideAttribute(key, plugin); + } + }.bind(this)); + }, + + _initializePlugin: function (plugin) { + this.applyOverrides(plugin); + if (typeof plugin.initialize === "function") { + plugin.initialize.bind(plugin)(this); + } + this.initialized_plugins.push(plugin.__name__); + }, + + asyncInitializePlugin: function (plugin) { + var deferred = new $.Deferred(); + this.loadOptionalDependencies(plugin.optional_dependencies).then( + _.compose( + deferred.resolve, + this._initializePlugin.bind(this), + _.partial(this.setOptionalDependencies, plugin) + )); + return deferred.promise(); + }, + + initializePlugin: function (plugin) { + var deferred = new $.Deferred(); + if (_.contains(this.initialized_plugins, plugin.__name__)) { + // Don't initialize plugins twice, otherwise we get + // infinite recursion in overridden methods. + return deferred.resolve().promise(); + } + _.extend(plugin, this.properties); + if (plugin.optional_dependencies) { + this.asyncInitializePlugin(plugin).then(deferred.resolve); + } else { + this._initializePlugin(plugin); + deferred.resolve(); + } + return deferred.promise(); + }, + + initNextPlugin: function (remaining, deferred) { + if (remaining.length === 0) { + deferred.resolve(); + return; + } + var plugin = remaining.pop(); + this.initializePlugin(plugin).then( + this.initNextPlugin.bind(this, remaining, deferred)); + }, + + initializePlugins: function (properties) { + /* The properties variable is an object of attributes and methods + * which will be attached to the plugins. + */ + var deferred = new $.Deferred(); + if (!_.size(this.plugins)) { + return deferred.promise(); + } + this.properties = properties; + this.initNextPlugin(_.values(this.plugins).reverse(), deferred); + return deferred; + } + }); + return { + 'enable': function (object) { + /* Call this method to make an object pluggable */ + return _.extend(object, {'pluggable': new Pluggable(object)}); + } + }; +})); diff --git a/src/converse-register.js b/src/converse-register.js index f784a1755..3778e892c 100644 --- a/src/converse-register.js +++ b/src/converse-register.js @@ -40,7 +40,7 @@ Strophe.Status.CONFLICT = i + 3; Strophe.Status.NOTACCEPTABLE = i + 5; - converse_api.plugins.add('register', { + converse_api.plugins.add('converse-register', { overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/converse-vcard.js b/src/converse-vcard.js index bb40eec35..4efb67584 100644 --- a/src/converse-vcard.js +++ b/src/converse-vcard.js @@ -20,7 +20,7 @@ moment = converse_api.env.moment; - converse_api.plugins.add('vcard', { + converse_api.plugins.add('converse-vcard', { overrides: { // Overrides mentioned here will be picked up by converse.js's diff --git a/src/utils.js b/src/utils.js index 957f6650c..dfc9194bd 100755 --- a/src/utils.js +++ b/src/utils.js @@ -149,7 +149,6 @@ return str; }, - isOTRMessage: function (message) { var $body = $(message).children('body'), text = ($body.length > 0 ? $body.text() : undefined); From 454f8ef034635e4fedbc782c9ac7908ae31b12b7 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Thu, 9 Jun 2016 21:22:34 +0000 Subject: [PATCH 2/7] Let converse.initialize return a deferred and use that in the tests, instead of a callback. --- src/converse-api.js | 2 +- src/converse-core.js | 29 +++++++++++------------------ tests/main.js | 4 +--- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/converse-api.js b/src/converse-api.js index b08ff1638..326c7a023 100644 --- a/src/converse-api.js +++ b/src/converse-api.js @@ -23,7 +23,7 @@ var Strophe = strophe.Strophe; return { 'initialize': function (settings, callback) { - converse.initialize(settings, callback); + return converse.initialize(settings, callback); }, 'log': converse.log, 'connection': { diff --git a/src/converse-core.js b/src/converse-core.js index 594449104..f8b6807e9 100755 --- a/src/converse-core.js +++ b/src/converse-core.js @@ -132,6 +132,7 @@ converse.initialize = function (settings, callback) { "use strict"; + var init_deferred = new $.Deferred(); var converse = this; var unloadevent; if ('onpagehide' in window) { @@ -621,7 +622,7 @@ }; - this.onStatusInitialized = function (deferred) { + this.onStatusInitialized = function () { this.registerIntervalHandler(); this.roster = new this.RosterContacts(); this.roster.browserStorage = new Backbone.BrowserStorage[this.storage]( @@ -629,20 +630,9 @@ this.chatboxes.onConnected(); this.giveFeedback(__('Contacts')); if (typeof this.callback === 'function') { - // A callback method may be passed in via the - // converse.initialize method. - // XXX: Can we use $.Deferred instead of this callback? - if (this.connection.service === 'jasmine tests') { - // XXX: Call back with the internal converse object. This - // object should never be exposed to production systems. - // 'jasmine tests' is an invalid http bind service value, - // so we're sure that this is just for tests. - this.callback(this); - } else { - this.callback(); - } + // XXX: Deprecate in favor of init_deferred + this.callback(); } - deferred.resolve(); converse.emit('initialized'); }; @@ -650,7 +640,6 @@ // When reconnecting, there might be some open chat boxes. We don't // know whether these boxes are of the same account or not, so we // close them now. - var deferred = new $.Deferred(); // XXX: ran into an issue where a returned PubSub BOSH response was // not received by the browser. The solution was to flush the // connection early on. I don't know what the underlying cause of @@ -670,13 +659,11 @@ this.domain = Strophe.getDomainFromJid(this.connection.jid); this.features = new this.Features(); this.enableCarbons(); - this.initStatus().done(_.bind(this.onStatusInitialized, this, deferred)); + this.initStatus().done(_.bind(this.onStatusInitialized, this)); converse.emit('connected'); converse.emit('ready'); // BBB: Will be removed. - return deferred.promise(); }; - this.RosterContact = Backbone.Model.extend({ initialize: function (attributes, options) { @@ -1796,7 +1783,13 @@ }).then(function () { converse._initialize(); converse.registerGlobalEventHandlers(); + if (converse.connection.service === 'jasmine tests') { + init_deferred.resolve(converse); + } else { + init_deferred.resolve(); + } }); + return init_deferred.promise(); }; return converse; })); diff --git a/tests/main.js b/tests/main.js index a244572a6..f34e7f6ed 100644 --- a/tests/main.js +++ b/tests/main.js @@ -56,7 +56,7 @@ require([ jid: 'dummy@localhost', password: 'secret', debug: true - }, function (converse) { + }).then(function (converse) { window.converse = converse; window.crypto = { getRandomValues: function (buf) { @@ -86,8 +86,6 @@ require([ "spec/register", "spec/xmppstatus", ], function () { - // Make sure this callback is only called once. - delete converse.callback; // Stub the trimChat method. It causes havoc when running with // phantomJS. converse.ChatBoxViews.prototype.trimChat = function () {}; From 1bf8b80cec0545ce893ea09290572809fe1e6d77 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Thu, 9 Jun 2016 22:05:55 +0000 Subject: [PATCH 3/7] Refactored converse-pluggable to remove all deferreds by not attempting to load `optional_dependencies` via require.js. Instead, we just expect them to be plugins and to have been loaded already. --- src/converse-core.js | 15 +++--- src/converse-muc.js | 9 ++-- src/converse-pluggable.js | 97 +++++++++++---------------------------- 3 files changed, 39 insertions(+), 82 deletions(-) diff --git a/src/converse-core.js b/src/converse-core.js index f8b6807e9..4d200cdcc 100755 --- a/src/converse-core.js +++ b/src/converse-core.js @@ -633,6 +633,11 @@ // XXX: Deprecate in favor of init_deferred this.callback(); } + if (converse.connection.service === 'jasmine tests') { + init_deferred.resolve(converse); + } else { + init_deferred.resolve(); + } converse.emit('initialized'); }; @@ -1780,15 +1785,9 @@ converse.pluggable.initializePlugins({ 'updateSettings': updateSettings, 'converse': converse - }).then(function () { - converse._initialize(); - converse.registerGlobalEventHandlers(); - if (converse.connection.service === 'jasmine tests') { - init_deferred.resolve(converse); - } else { - init_deferred.resolve(); - } }); + converse._initialize(); + converse.registerGlobalEventHandlers(); return init_deferred.promise(); }; return converse; diff --git a/src/converse-muc.js b/src/converse-muc.js index adc05e22f..f565fb6a0 100755 --- a/src/converse-muc.js +++ b/src/converse-muc.js @@ -43,14 +43,13 @@ Strophe.addNamespace('MUC_USER', Strophe.NS.MUC + "#user"); converse_api.plugins.add('converse-muc', { - /* Optional dependencies are require.js dependencies which might be - * overridden or relied upon if they exist, but otherwise ignored. + /* Optional dependencies are other plugins which might be + * overridden or relied upon, if they exist, otherwise they're ignored. * * However, if the setting "strict_plugin_dependencies" is set to true, - * then these dependencies will be considered required. + * an error will be raised if the plugin is not found. * - * Optional dependencies will be available in the initialize method as - * a the "optional_dependencies" attribute of the plugin. + * NB: These plugins need to have already been loaded via require.js. */ optional_dependencies: ["converse-controlbox"], diff --git a/src/converse-pluggable.js b/src/converse-pluggable.js index 41f273815..783ae8ad4 100644 --- a/src/converse-pluggable.js +++ b/src/converse-pluggable.js @@ -84,33 +84,23 @@ }.bind(this)); }, - setOptionalDependencies: function (plugin, dependencies) { - plugin.optional_dependencies = dependencies; - return plugin; - }, - - loadOptionalDependencies: function (plugins) { - var deferred = new $.Deferred(); - require(plugins, - function () { - _.each(plugins, function (name) { - var plugin = this.plugins[name]; - if (plugin) { - this.initializePlugin(plugin).then( - deferred.resolve.bind(this, plugins) - ); - } - }.bind(this)); - }.bind(this), - function () { - if (this.plugged.strict_plugin_dependencies) { - deferred.fail.apply(this, arguments); - this.throwUndefinedDependencyError(arguments[0]); - } else { - deferred.resolve.apply(this, [plugins]); + loadOptionalDependencies: function (plugin) { + _.each(plugin.optional_dependencies, function (name) { + var dep = this.plugins[name]; + if (dep) { + if (_.contains(dep.optional_dependencies, plugin.__name__)) { + // FIXME: circular dependency checking is only one level deep. + throw "Found a circular dependency between the plugins \""+ + plugin.__name__+"\" and \""+name+"\""; } - }.bind(this)); - return deferred.promise(); + this.initializePlugin(dep); + } else { + this.throwUndefinedDependencyError( + "Could not find optional dependency \""+name+"\" "+ + "for the plugin \""+plugin.__name__+"\". "+ + "If it's needed, make sure it's loaded by require.js"); + } + }.bind(this)); }, throwUndefinedDependencyError: function (msg) { @@ -140,7 +130,16 @@ }.bind(this)); }, - _initializePlugin: function (plugin) { + initializePlugin: function (plugin) { + if (_.contains(this.initialized_plugins, plugin.__name__)) { + // Don't initialize plugins twice, otherwise we get + // infinite recursion in overridden methods. + return; + } + _.extend(plugin, this.properties); + if (plugin.optional_dependencies) { + this.loadOptionalDependencies(plugin); + } this.applyOverrides(plugin); if (typeof plugin.initialize === "function") { plugin.initialize.bind(plugin)(this); @@ -148,55 +147,15 @@ this.initialized_plugins.push(plugin.__name__); }, - asyncInitializePlugin: function (plugin) { - var deferred = new $.Deferred(); - this.loadOptionalDependencies(plugin.optional_dependencies).then( - _.compose( - deferred.resolve, - this._initializePlugin.bind(this), - _.partial(this.setOptionalDependencies, plugin) - )); - return deferred.promise(); - }, - - initializePlugin: function (plugin) { - var deferred = new $.Deferred(); - if (_.contains(this.initialized_plugins, plugin.__name__)) { - // Don't initialize plugins twice, otherwise we get - // infinite recursion in overridden methods. - return deferred.resolve().promise(); - } - _.extend(plugin, this.properties); - if (plugin.optional_dependencies) { - this.asyncInitializePlugin(plugin).then(deferred.resolve); - } else { - this._initializePlugin(plugin); - deferred.resolve(); - } - return deferred.promise(); - }, - - initNextPlugin: function (remaining, deferred) { - if (remaining.length === 0) { - deferred.resolve(); - return; - } - var plugin = remaining.pop(); - this.initializePlugin(plugin).then( - this.initNextPlugin.bind(this, remaining, deferred)); - }, - initializePlugins: function (properties) { /* The properties variable is an object of attributes and methods * which will be attached to the plugins. */ - var deferred = new $.Deferred(); if (!_.size(this.plugins)) { - return deferred.promise(); + return; } this.properties = properties; - this.initNextPlugin(_.values(this.plugins).reverse(), deferred); - return deferred; + _.each(_.values(this.plugins), this.initializePlugin.bind(this)); } }); return { From 6258cfa089894d883f2d8ab4fc7ef11b8aeda3bf Mon Sep 17 00:00:00 2001 From: JC Brand Date: Thu, 9 Jun 2016 21:25:19 +0000 Subject: [PATCH 4/7] Check the "closed" state of the controlbox when trimming chats. --- src/converse-minimize.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/converse-minimize.js b/src/converse-minimize.js index 45e358a0f..a2e9d7b0e 100644 --- a/src/converse-minimize.js +++ b/src/converse-minimize.js @@ -218,7 +218,14 @@ getShownChats: function () { return this.filter(function (view) { - return (!view.model.get('minimized') && view.$el.is(':visible')); + // The controlbox can take a while to close, + // so we need to check its state. That's why we checked + // the 'closed' state. + return ( + !view.model.get('minimized') && + !view.model.get('closed') && + view.$el.is(':visible') + ); }); }, From 854633089d2a2e204a7168272e21a75612271c75 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 10 Jun 2016 16:13:52 +0200 Subject: [PATCH 5/7] Add config setting to disable MUC direct invites. --- docs/CHANGES.md | 2 ++ docs/source/configuration.rst | 9 ++++++++ src/converse-muc.js | 35 ++++++++++++++++++----------- src/templates/chatroom_sidebar.html | 2 ++ 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/docs/CHANGES.md b/docs/CHANGES.md index 1b1e15385..dc6264bdf 100755 --- a/docs/CHANGES.md +++ b/docs/CHANGES.md @@ -2,6 +2,7 @@ ## 1.0.3 (Unreleased) +- Update the plugin architecture to allow plugins to have optional dependencies [jcbrand] - Bugfix. Login form doesn't render after logging out, when `auto_reconnect = false` [jcbrand] - Also indicate new day for the first day's messages. [jcbrand] - Chat bot messages don't appear when they have the same ids as their commands. [jcbrand] @@ -11,6 +12,7 @@ - Don't use sound and desktop notifications for OTR messages (when setting up the session) [jcbrand] - New config option [default_state](https://conversejs.org/docs/html/configuration.html#default_state) [jcbrand] - New API method `converse.rooms.close()` +- New configuration setting [allow_muc_invites](https://conversejs.org/docs/html/configuration.html#allow-muc-invites) [jcbrand] - #553 Add processing hints to OTR messages [jcbrand] - #650 Don't ignore incoming messages with same JID as current user (might be MAM archived) [jcbrand] diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index d0e61a357..8cc158df9 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -141,6 +141,15 @@ allow_muc Allow multi-user chat (muc) in chatrooms. Setting this to ``false`` will remove the ``Chatrooms`` tab from the control box. +allow_muc_invitations +--------------------- + +* Default: ``true`` + +Allows users to be invited to join MUC chat rooms. An "Invite" widget will +appear in the sidebar of the chat room where you can type in the JID of a user +to invite into the chat room. + allow_otr --------- diff --git a/src/converse-muc.js b/src/converse-muc.js index f565fb6a0..ce42eb341 100755 --- a/src/converse-muc.js +++ b/src/converse-muc.js @@ -74,9 +74,11 @@ Features: { addClientFeatures: function () { this._super.addClientFeatures.apply(this, arguments); - converse.connection.disco.addFeature('jabber:x:conference'); // Invites - if (this.allow_muc) { - this.connection.disco.addFeature(Strophe.NS.MUC); + if (converse.allow_muc_invitations) { + converse.connection.disco.addFeature('jabber:x:conference'); // Invites + } + if (converse.allow_muc) { + converse.connection.disco.addFeature(Strophe.NS.MUC); } } }, @@ -155,6 +157,7 @@ var converse = this.converse; // Configuration values for this plugin this.updateSettings({ + allow_muc_invitations: true, allow_muc: true, auto_join_on_invite: false, // Auto-join chatroom on invite auto_join_rooms: [], // List of maps {'jid': 'room@example.org', 'nick': 'WizardKing69' }, @@ -966,11 +969,15 @@ render: function () { this.$el.html( converse.templates.chatroom_sidebar({ + 'allow_muc_invitations': converse.allow_muc_invitations, 'label_invitation': __('Invite'), 'label_occupants': __('Occupants') }) ); - return this.initInviteWidget(); + if (converse.allow_muc_invitations) { + return this.initInviteWidget(); + } + return this; }, onOccupantAdded: function (item) { @@ -1362,15 +1369,17 @@ }; converse.on('chatBoxesFetched', autoJoinRooms); - var onConnected = function () { - converse.connection.addHandler( - function (message) { - converse.onDirectMUCInvitation(message); - return true; - }, 'jabber:x:conference', 'message'); - }; - converse.on('connected', onConnected); - converse.on('reconnected', onConnected); + if (converse.allow_muc_invitations) { + var onConnected = function () { + converse.connection.addHandler( + function (message) { + converse.onDirectMUCInvitation(message); + return true; + }, 'jabber:x:conference', 'message'); + }; + converse.on('connected', onConnected); + converse.on('reconnected', onConnected); + } /* ------------------------------------------------------------ */ diff --git a/src/templates/chatroom_sidebar.html b/src/templates/chatroom_sidebar.html index 3b8e5be28..656c6db9a 100644 --- a/src/templates/chatroom_sidebar.html +++ b/src/templates/chatroom_sidebar.html @@ -1,7 +1,9 @@ +{[ if (allow_muc_invitations) { ]}
+{[ } ]}

{{label_occupants}}:

    From 380a60aebd896250cf839431d742a1316bbf8106 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 10 Jun 2016 15:05:49 +0000 Subject: [PATCH 6/7] Two fixes related to occupants toggling * Empty chat area doesn't resize when hiding occupants. * Properly change icon when toggling occupants --- css/converse.css | 4 ++-- sass/_chatrooms.scss | 4 ++-- src/converse-muc.js | 5 ++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/css/converse.css b/css/converse.css index 9477ca8a6..632463d11 100644 --- a/css/converse.css +++ b/css/converse.css @@ -2004,9 +2004,9 @@ #conversejs .chatroom .box-flyout .chatroom-body .chat-area .chat-content { padding: 0 0.5em 0 0.5em; } #conversejs .chatroom .box-flyout .chatroom-body .chat-area.full { - max-width: 100%; } + min-width: 100%; } #conversejs .chatroom .box-flyout .chatroom-body .chat-area.full .new-msgs-indicator { - max-width: 100%; } + min-width: 100%; } #conversejs .chatroom .box-flyout .chatroom-body .mentioned { font-weight: bold; } #conversejs .chatroom .box-flyout .chatroom-body .chat-msg-room { diff --git a/sass/_chatrooms.scss b/sass/_chatrooms.scss index 11c286fec..54e0ff2d4 100644 --- a/sass/_chatrooms.scss +++ b/sass/_chatrooms.scss @@ -59,9 +59,9 @@ padding: 0 0.5em 0 0.5em; } &.full { - max-width: 100%; + min-width: 100%; .new-msgs-indicator { - max-width: 100%; + min-width: 100%; } } } diff --git a/src/converse-muc.js b/src/converse-muc.js index ce42eb341..5a570b539 100755 --- a/src/converse-muc.js +++ b/src/converse-muc.js @@ -263,16 +263,15 @@ // Bit of a hack, to make sure that the sidebar's state doesn't change this.model.set({hidden_occupants: !this.model.get('hidden_occupants')}); } - var $el = this.$('.icon-hide-users'); if (!this.model.get('hidden_occupants')) { this.model.save({hidden_occupants: true}); - $el.removeClass('icon-hide-users').addClass('icon-show-users'); + this.$('.icon-hide-users').removeClass('icon-hide-users').addClass('icon-show-users'); this.$('.occupants').addClass('hidden'); this.$('.chat-area').addClass('full'); this.scrollDown(); } else { this.model.save({hidden_occupants: false}); - $el.removeClass('icon-show-users').addClass('icon-hide-users'); + this.$('.icon-show-users').removeClass('icon-show-users').addClass('icon-hide-users'); this.$('.chat-area').removeClass('full'); this.$('div.occupants').removeClass('hidden'); this.scrollDown(); From e2f1c68cff25b9c734b8252005e9a18ced356bdb Mon Sep 17 00:00:00 2001 From: JC Brand Date: Sat, 11 Jun 2016 07:41:52 +0000 Subject: [PATCH 7/7] Always show the chat when calling 'open' --- src/converse-muc.js | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/converse-muc.js b/src/converse-muc.js index 5a570b539..7d7a4b18a 100755 --- a/src/converse-muc.js +++ b/src/converse-muc.js @@ -1413,18 +1413,14 @@ } var _transform = function (jid) { jid = jid.toLowerCase(); - var chatroom = converse.chatboxes.get(jid); - if (!chatroom) { - chatroom = converse.chatboxviews.showChat({ - 'id': jid, - 'jid': jid, - 'name': Strophe.unescapeNode(Strophe.getNodeFromJid(jid)), - 'nick': nick, - 'type': 'chatroom', - 'box_id': b64_sha1(jid) - }); - } - return converse.wrappedChatBox(converse.chatboxes.getChatBox(jid, true)); + return converse.wrappedChatBox(converse.chatboxviews.showChat({ + 'id': jid, + 'jid': jid, + 'name': Strophe.unescapeNode(Strophe.getNodeFromJid(jid)), + 'nick': nick, + 'type': 'chatroom', + 'box_id': b64_sha1(jid) + })); }; if (typeof jids === "undefined") { throw new TypeError('rooms.open: You need to provide at least one JID');