From 3fe19453475e33f17f79fa47148e109295dae98c Mon Sep 17 00:00:00 2001 From: Antony Lesuisse Date: Sat, 10 Nov 2012 20:00:14 +0100 Subject: [PATCH 1/3] use ES5 bind() for better stack traces bzr revid: al@openerp.com-20121110190014-ead1981ez6up6ri1 --- addons/web/static/src/js/corelib.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/addons/web/static/src/js/corelib.js b/addons/web/static/src/js/corelib.js index d2e94d37939..f2fb1bb2141 100644 --- a/addons/web/static/src/js/corelib.js +++ b/addons/web/static/src/js/corelib.js @@ -429,17 +429,11 @@ instance.web.PropertiesMixin = _.extend({}, instance.web.EventDispatcherMixin, { instance.web.CallbackEnabledMixin = _.extend({}, instance.web.PropertiesMixin, { init: function() { instance.web.PropertiesMixin.init.call(this); - var self = this; // Transform on_/do_* methods into callbacks - var callback_maker = function(fn) { - return function() { - return fn.apply(self, arguments); - } - }; for (var name in this) { if(typeof(this[name]) == "function") { if((/^on_|^do_/).test(name)) { - this[name] = callback_maker(this[name]); + this[name] = this[name].bind(this); } } } From 27c872cef382ab18532b55fd675fb1e6f316d568 Mon Sep 17 00:00:00 2001 From: Antony Lesuisse Date: Sat, 10 Nov 2012 20:34:40 +0100 Subject: [PATCH 2/3] remove CallbackEnabled The on_/do_ binding still happens in Widget.init, but as we now use ES5 bind() it doesnt clutter stack traces anymore. However the fate of this automatic binding feature remains uncertain. bzr revid: al@openerp.com-20121110193440-78mpwamqx7iwq2ux --- addons/web/static/src/js/chrome.js | 2 +- addons/web/static/src/js/corelib.js | 110 +++++++++++--------------- addons/web/static/src/js/coresetup.js | 16 ++-- addons/web/static/src/js/data.js | 4 +- addons/web/static/src/js/view_list.js | 3 +- 5 files changed, 59 insertions(+), 76 deletions(-) diff --git a/addons/web/static/src/js/chrome.js b/addons/web/static/src/js/chrome.js index 9c249a5fb0c..99c7a73f3bd 100644 --- a/addons/web/static/src/js/chrome.js +++ b/addons/web/static/src/js/chrome.js @@ -189,7 +189,7 @@ instance.web.Dialog = instance.web.Widget.extend({ } }); -instance.web.CrashManager = instance.web.CallbackEnabled.extend({ +instance.web.CrashManager = instance.web.Class.extend({ rpc_error: function(error) { if (error.data.fault_code) { var split = ("" + error.data.fault_code).split('\n')[0].split(' -- '); diff --git a/addons/web/static/src/js/corelib.js b/addons/web/static/src/js/corelib.js index f2fb1bb2141..dab06b374e9 100644 --- a/addons/web/static/src/js/corelib.js +++ b/addons/web/static/src/js/corelib.js @@ -243,15 +243,17 @@ instance.web.ParentedMixin = { /** * Backbone's events. Do not ever use it directly, use EventDispatcherMixin instead. + * + * This class just handle the dispatching of events, it is not meant to be extended, + * nor used directly. All integration with parenting and automatic unregistration of + * events is done in EventDispatcherMixin. + * + * Copyright notice for the following Class: * * (c) 2010-2012 Jeremy Ashkenas, DocumentCloud Inc. * Backbone may be freely distributed under the MIT license. * For all details and documentation: * http://backbonejs.org - * - * This class just handle the dispatching of events, it is not meant to be extended, - * nor used directly. All integration with parenting and automatic unregistration of - * events is done in EventDispatcherMixin. * */ var Events = instance.web.Class.extend({ @@ -334,7 +336,6 @@ var Events = instance.web.Class.extend({ return this; } }); -// end of Jeremy Ashkenas' code instance.web.EventDispatcherMixin = _.extend({}, instance.web.ParentedMixin, { __eventDispatcherMixin: true, @@ -426,10 +427,21 @@ instance.web.PropertiesMixin = _.extend({}, instance.web.EventDispatcherMixin, { } }); -instance.web.CallbackEnabledMixin = _.extend({}, instance.web.PropertiesMixin, { - init: function() { +instance.web.WidgetMixin = _.extend({},instance.web.PropertiesMixin, { + /** + * Constructs the widget and sets its parent if a parent is given. + * + * @constructs instance.web.Widget + * + * @param {instance.web.Widget} parent Binds the current instance to the given Widget instance. + * When that widget is destroyed by calling destroy(), the current instance will be + * destroyed too. Can be null. + */ + init: function(parent) { instance.web.PropertiesMixin.init.call(this); - // Transform on_/do_* methods into callbacks + this.setParent(parent); + // Bind on_/do_* methods to this + // We might remove this automatic binding in the future for (var name in this) { if(typeof(this[name]) == "function") { if((/^on_|^do_/).test(name)) { @@ -438,49 +450,6 @@ instance.web.CallbackEnabledMixin = _.extend({}, instance.web.PropertiesMixin, { } } }, - /** - * Proxies a method of the object, in order to keep the right ``this`` on - * method invocations. - * - * This method is similar to ``Function.prototype.bind`` or ``_.bind``, and - * even more so to ``jQuery.proxy`` with a fundamental difference: its - * resolution of the method being called is lazy, meaning it will use the - * method as it is when the proxy is called, not when the proxy is created. - * - * Other methods will fix the bound method to what it is when creating the - * binding/proxy, which is fine in most javascript code but problematic in - * OpenERP Web where developers may want to replace existing callbacks with - * theirs. - * - * The semantics of this precisely replace closing over the method call. - * - * @param {String|Function} method function or name of the method to invoke - * @returns {Function} proxied method - */ - proxy: function (method) { - var self = this; - return function () { - var fn = (typeof method === 'string') ? self[method] : method; - return fn.apply(self, arguments); - } - } -}); - -instance.web.WidgetMixin = _.extend({},instance.web.CallbackEnabledMixin, { - /** - * Constructs the widget and sets its parent if a parent is given. - * - * @constructs instance.web.Widget - * @extends instance.web.CallbackEnabled - * - * @param {instance.web.Widget} parent Binds the current instance to the given Widget instance. - * When that widget is destroyed by calling destroy(), the current instance will be - * destroyed too. Can be null. - */ - init: function(parent) { - instance.web.CallbackEnabledMixin.init.call(this); - this.setParent(parent); - }, /** * Destroys the current widget, also destroys all its children before destroying itself. */ @@ -568,17 +537,36 @@ instance.web.WidgetMixin = _.extend({},instance.web.CallbackEnabledMixin, { */ start: function() { return $.when(); + }, + /** + * Proxies a method of the object, in order to keep the right ``this`` on + * method invocations. + * + * This method is similar to ``Function.prototype.bind`` or ``_.bind``, and + * even more so to ``jQuery.proxy`` with a fundamental difference: its + * resolution of the method being called is lazy, meaning it will use the + * method as it is when the proxy is called, not when the proxy is created. + * + * Other methods will fix the bound method to what it is when creating the + * binding/proxy, which is fine in most javascript code but problematic in + * OpenERP Web where developers may want to replace existing callbacks with + * theirs. + * + * The semantics of this precisely replace closing over the method call. + * + * @param {String|Function} method function or name of the method to invoke + * @returns {Function} proxied method + */ + proxy: function (method) { + var self = this; + return function () { + var fn = (typeof method === 'string') ? self[method] : method; + return fn.apply(self, arguments); + } } }); // Classes - -instance.web.CallbackEnabled = instance.web.Class.extend(instance.web.CallbackEnabledMixin, { - init: function() { - instance.web.CallbackEnabledMixin.init.call(this); - } -}); - /** * Base class for all visual components. Provides a lot of functionalities helpful * for the management of a part of the DOM. @@ -645,7 +633,6 @@ instance.web.Widget = instance.web.Class.extend(instance.web.WidgetMixin, { * Constructs the widget and sets its parent if a parent is given. * * @constructs instance.web.Widget - * @extends instance.web.CallbackEnabled * * @param {instance.web.Widget} parent Binds the current instance to the given Widget instance. * When that widget is destroyed by calling destroy(), the current instance will be @@ -935,7 +922,7 @@ instance.web.Registry = instance.web.Class.extend({ } }); -instance.web.JsonRPC = instance.web.CallbackEnabled.extend({ +instance.web.JsonRPC = instance.web.Class.extend(instance.web.PropertiesMixin, { triggers: { 'request': 'Request sent', 'response': 'Response received', @@ -944,13 +931,12 @@ instance.web.JsonRPC = instance.web.CallbackEnabled.extend({ }, /** * @constructs instance.web.JsonRPC - * @extends instance.web.CallbackEnabled * * @param {String} [server] JSON-RPC endpoint hostname * @param {String} [port] JSON-RPC endpoint port */ init: function() { - this._super(); + instance.web.PropertiesMixin.init.call(this); this.server = null; this.debug = ($.deparam($.param.querystring()).debug != undefined); }, diff --git a/addons/web/static/src/js/coresetup.js b/addons/web/static/src/js/coresetup.js index b7dac303520..f8a30e848ba 100644 --- a/addons/web/static/src/js/coresetup.js +++ b/addons/web/static/src/js/coresetup.js @@ -53,7 +53,7 @@ instance.web.Session = instance.web.JsonRPC.extend( /** @lends instance.web.Sess this.session_id = this.get_cookie('session_id'); return this.session_reload().then(function(result) { var modules = instance._modules.join(','); - var deferred = self.rpc('/web/webclient/qweblist', {mods: modules}).then(self.do_load_qweb); + var deferred = self.rpc('/web/webclient/qweblist', {mods: modules}).then(self.load_qweb.bind(self)); if(self.session_is_valid()) { return deferred.then(function() { return self.load_modules(); }); } @@ -168,15 +168,15 @@ instance.web.Session = instance.web.JsonRPC.extend( /** @lends instance.web.Sess if(to_load.length) { loaded = $.when( loaded, - self.rpc('/web/webclient/csslist', {mods: to_load}).done(self.do_load_css), - self.rpc('/web/webclient/qweblist', {mods: to_load}).then(self.do_load_qweb), + self.rpc('/web/webclient/csslist', {mods: to_load}).done(self.load_css.bind(self)), + self.rpc('/web/webclient/qweblist', {mods: to_load}).then(self.load_qweb.bind(self)), self.rpc('/web/webclient/jslist', {mods: to_load}).done(function(files) { file_list = file_list.concat(files); }) ); } return loaded.then(function () { - return self.do_load_js(file_list); + return self.load_js(file_list); }).done(function() { self.on_modules_loaded(); self.trigger('module_loaded'); @@ -190,7 +190,7 @@ instance.web.Session = instance.web.JsonRPC.extend( /** @lends instance.web.Sess }); }); }, - do_load_css: function (files) { + load_css: function (files) { var self = this; _.each(files, function (file) { $('head').append($('', { @@ -200,7 +200,7 @@ instance.web.Session = instance.web.JsonRPC.extend( /** @lends instance.web.Sess })); }); }, - do_load_js: function(files) { + load_js: function(files) { var self = this; var d = $.Deferred(); if(files.length != 0) { @@ -212,7 +212,7 @@ instance.web.Session = instance.web.JsonRPC.extend( /** @lends instance.web.Sess if ( (tag.readyState && tag.readyState != "loaded" && tag.readyState != "complete") || tag.onload_done ) return; tag.onload_done = true; - self.do_load_js(files).done(function () { + self.load_js(files).done(function () { d.resolve(); }); }; @@ -223,7 +223,7 @@ instance.web.Session = instance.web.JsonRPC.extend( /** @lends instance.web.Sess } return d; }, - do_load_qweb: function(files) { + load_qweb: function(files) { var self = this; _.each(files, function(file) { self.qweb_mutex.exec(function() { diff --git a/addons/web/static/src/js/data.js b/addons/web/static/src/js/data.js index 2f8679794b0..5bd7267b4a9 100644 --- a/addons/web/static/src/js/data.js +++ b/addons/web/static/src/js/data.js @@ -358,17 +358,15 @@ instance.web.Model = instance.web.Class.extend({ }, }); -instance.web.DataSet = instance.web.CallbackEnabled.extend({ +instance.web.DataSet = instance.web.Class.extend({ /** * Collection of OpenERP records, used to share records and the current selection between views. * * @constructs instance.web.DataSet - * @extends instance.web.CallbackEnabled * * @param {String} model the OpenERP model this dataset will manage */ init: function(parent, model, context) { - this._super(parent); this.model = model; this.context = context || {}; this.index = null; diff --git a/addons/web/static/src/js/view_list.js b/addons/web/static/src/js/view_list.js index 6779b6fe841..97efb983235 100644 --- a/addons/web/static/src/js/view_list.js +++ b/addons/web/static/src/js/view_list.js @@ -1557,9 +1557,8 @@ instance.web.ListView.Groups = instance.web.Class.extend( /** @lends instance.we } }); -var DataGroup = instance.web.CallbackEnabled.extend({ +var DataGroup = instance.web.Class.extend({ init: function(parent, model, domain, context, group_by, level) { - this._super(parent, null); this.model = new instance.web.Model(model, context, domain); this.group_by = group_by; this.context = context; From 2e810433c9a144434319051784cc1038623536d2 Mon Sep 17 00:00:00 2001 From: Antony Lesuisse Date: Sat, 10 Nov 2012 20:46:54 +0100 Subject: [PATCH 3/3] remove WidgetMixin bzr revid: al@openerp.com-20121110194654-0q10uajsy6nmzzd6 --- addons/web/static/src/js/corelib.js | 149 +++++++++++++--------------- 1 file changed, 67 insertions(+), 82 deletions(-) diff --git a/addons/web/static/src/js/corelib.js b/addons/web/static/src/js/corelib.js index dab06b374e9..4012be526e6 100644 --- a/addons/web/static/src/js/corelib.js +++ b/addons/web/static/src/js/corelib.js @@ -427,7 +427,70 @@ instance.web.PropertiesMixin = _.extend({}, instance.web.EventDispatcherMixin, { } }); -instance.web.WidgetMixin = _.extend({},instance.web.PropertiesMixin, { +// Classes + +/** + * Base class for all visual components. Provides a lot of functionalities helpful + * for the management of a part of the DOM. + * + * Widget handles: + * - Rendering with QWeb. + * - Life-cycle management and parenting (when a parent is destroyed, all its children are + * destroyed too). + * - Insertion in DOM. + * + * Guide to create implementations of the Widget class: + * ============================================== + * + * Here is a sample child class: + * + * MyWidget = instance.base.Widget.extend({ + * // the name of the QWeb template to use for rendering + * template: "MyQWebTemplate", + * + * init: function(parent) { + * this._super(parent); + * // stuff that you want to init before the rendering + * }, + * start: function() { + * // stuff you want to make after the rendering, `this.$el` holds a correct value + * this.$el.find(".my_button").click(/* an example of event binding * /); + * + * // if you have some asynchronous operations, it's a good idea to return + * // a promise in start() + * var promise = this.rpc(...); + * return promise; + * } + * }); + * + * Now this class can simply be used with the following syntax: + * + * var my_widget = new MyWidget(this); + * my_widget.appendTo($(".some-div")); + * + * With these two lines, the MyWidget instance was inited, rendered, it was inserted into the + * DOM inside the ".some-div" div and its events were binded. + * + * And of course, when you don't need that widget anymore, just do: + * + * my_widget.destroy(); + * + * That will kill the widget in a clean way and erase its content from the dom. + */ +instance.web.Widget = instance.web.Class.extend(instance.web.PropertiesMixin, { + // Backbone-ish API + tagName: 'div', + id: null, + className: null, + attributes: {}, + events: {}, + /** + * The name of the QWeb template that will be used for rendering. Must be + * redefined in subclasses or the default render() method can not be used. + * + * @type string + */ + template: null, /** * Constructs the widget and sets its parent if a parent is given. * @@ -449,6 +512,9 @@ instance.web.WidgetMixin = _.extend({},instance.web.PropertiesMixin, { } } } + // FIXME: this should not be + this.setElement(this._make_descriptive()); + this.session = instance.session; }, /** * Destroys the current widget, also destroys all its children before destroying itself. @@ -563,86 +629,6 @@ instance.web.WidgetMixin = _.extend({},instance.web.PropertiesMixin, { var fn = (typeof method === 'string') ? self[method] : method; return fn.apply(self, arguments); } - } -}); - -// Classes -/** - * Base class for all visual components. Provides a lot of functionalities helpful - * for the management of a part of the DOM. - * - * Widget handles: - * - Rendering with QWeb. - * - Life-cycle management and parenting (when a parent is destroyed, all its children are - * destroyed too). - * - Insertion in DOM. - * - * Guide to create implementations of the Widget class: - * ============================================== - * - * Here is a sample child class: - * - * MyWidget = instance.base.Widget.extend({ - * // the name of the QWeb template to use for rendering - * template: "MyQWebTemplate", - * - * init: function(parent) { - * this._super(parent); - * // stuff that you want to init before the rendering - * }, - * start: function() { - * // stuff you want to make after the rendering, `this.$el` holds a correct value - * this.$el.find(".my_button").click(/* an example of event binding * /); - * - * // if you have some asynchronous operations, it's a good idea to return - * // a promise in start() - * var promise = this.rpc(...); - * return promise; - * } - * }); - * - * Now this class can simply be used with the following syntax: - * - * var my_widget = new MyWidget(this); - * my_widget.appendTo($(".some-div")); - * - * With these two lines, the MyWidget instance was inited, rendered, it was inserted into the - * DOM inside the ".some-div" div and its events were binded. - * - * And of course, when you don't need that widget anymore, just do: - * - * my_widget.destroy(); - * - * That will kill the widget in a clean way and erase its content from the dom. - */ -instance.web.Widget = instance.web.Class.extend(instance.web.WidgetMixin, { - // Backbone-ish API - tagName: 'div', - id: null, - className: null, - attributes: {}, - events: {}, - /** - * The name of the QWeb template that will be used for rendering. Must be - * redefined in subclasses or the default render() method can not be used. - * - * @type string - */ - template: null, - /** - * Constructs the widget and sets its parent if a parent is given. - * - * @constructs instance.web.Widget - * - * @param {instance.web.Widget} parent Binds the current instance to the given Widget instance. - * When that widget is destroyed by calling destroy(), the current instance will be - * destroyed too. Can be null. - */ - init: function(parent) { - instance.web.WidgetMixin.init.call(this,parent); - // FIXME: this should not be - this.setElement(this._make_descriptive()); - this.session = instance.session; }, /** * Renders the element. The default implementation renders the widget using QWeb, @@ -659,7 +645,6 @@ instance.web.Widget = instance.web.Class.extend(instance.web.WidgetMixin, { } this.replaceElement($el); }, - /** * Re-sets the widget's root element and replaces the old root element * (if any) by the new one in the DOM.