From 6bc75b61be1f8cd5c4e459bb5f8f2da7fa8ca29b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Mon, 3 Apr 2017 13:54:28 +0200 Subject: [PATCH] [FIX] web: better error handling in view and action manager Before this commit, it could happen that a crash at the wrong time (for example, while loading a view) cause the web client to crash for various reasons. For example, it assumes that the active view always has a valid controller. Or the action manager could not detach the previous view, so we had the 'double view' rendering in the web client. With this commit, we add some protections at various points. Those safety measures should help the user keep working even if a specific view did crash. --- .../web/static/src/js/chrome/action_manager.js | 18 ++++++++++++++++-- .../web/static/src/js/chrome/view_manager.js | 5 +++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/addons/web/static/src/js/chrome/action_manager.js b/addons/web/static/src/js/chrome/action_manager.js index 9221561f2d9..934473109a3 100644 --- a/addons/web/static/src/js/chrome/action_manager.js +++ b/addons/web/static/src/js/chrome/action_manager.js @@ -185,13 +185,17 @@ var ViewManagerAction = WidgetAction.extend({ * @param {int} [scrollTop] the number of pixels to scroll */ setScrollTop: function(scrollTop) { - this.widget.active_view.controller.setScrollTop(scrollTop); + var viewController = this.widget.active_view.controller; + if (viewController) { + viewController.setScrollTop(scrollTop); + } }, /** * @return {int} the number of pixels the webclient is scrolled when leaving the action */ getScrollTop: function() { - return this.widget.active_view.controller.getScrollTop(); + var viewController = this.widget.active_view.controller; + return viewController ? viewController.getScrollTop() : 0; }, /** * @return {Array} array of Objects that will be interpreted to display the breadcrumbs @@ -340,6 +344,16 @@ var ActionManager = Widget.extend({ // Hide the main ControlPanel for widgets that do not use it self.main_control_panel.do_hide(); } + + // most of the time, the self.$el element should already be empty, + // because we detached the old action just a few line up. However, + // it may happen that it is not empty, for example when a view + // manager was unable to load a view because of a crash. In any + // case, this is done as a safety measure to avoid the 'double view' + // situation that we had when the web client was unable to recover + // from a crash. + self.$el.empty(); + dom.append(self.$el, new_widget_fragment, { in_DOM: self.is_in_DOM, callbacks: [{widget: self.inner_widget}], diff --git a/addons/web/static/src/js/chrome/view_manager.js b/addons/web/static/src/js/chrome/view_manager.js index 02250d5cc15..d8f0e3776d9 100644 --- a/addons/web/static/src/js/chrome/view_manager.js +++ b/addons/web/static/src/js/chrome/view_manager.js @@ -59,7 +59,8 @@ var ViewManager = Widget.extend(ControlPanelMixin, { */ on_detach_callback: function() { this.is_in_DOM = false; - if (this.active_view && this.active_view.controller.on_detach_callback) { + var controller = this.active_view && this.active_view.controller; + if (controller && controller.on_detach_callback) { this.active_view.controller.on_detach_callback(); } }, @@ -282,7 +283,7 @@ var ViewManager = Widget.extend(ControlPanelMixin, { view.$fragment = view.$fragment.contents(); view.loaded.resolve(); }); - }); + }).fail(view.loaded.reject.bind(view.loaded)); } else { view.loaded = view.loaded.then(function() { view_options = _.extend({}, view_options, self.env);