From c995aa2cfd62f90baae9672e80d78b6554503e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20K=C3=BChn?= Date: Thu, 16 Aug 2018 13:31:02 +0200 Subject: [PATCH] [FIX] mail: tests: remove phantomjs hack with discuss Revision on https://github.com/odoo/odoo/commit/e0a51edd856b36bfdf1cacb4d7e8d70fad7884fa In the commit above, we added an option to Discuss widget to render threads specifically for PhantomJS. The reason for this is that a test was failing, because it needs full support of CSS flexbox to pass. However, PhantomJS does not support flexbox very well. As a result, it was messing with the rendering of threads. We couldn't make the hack just in the test itself, because it is already at the moment of rendering the thread that PhantomJS messes the whole process of the test. PhantomJS has been replaced with Chrome headless [1], So the hack is no longer be necessary, as Chrome headless fully supports CSS flexbox. [1] https://github.com/odoo/odoo/commit/de84ad9c1f4853cb8a748ee1d0c880081dae9fe2 --- addons/mail/static/src/js/discuss.js | 8 -------- addons/mail/static/src/xml/thread.xml | 10 ++-------- addons/mail/static/tests/discuss_tests.js | 3 +-- addons/mail/static/tests/helpers/test_utils.js | 8 ++------ 4 files changed, 5 insertions(+), 24 deletions(-) diff --git a/addons/mail/static/src/js/discuss.js b/addons/mail/static/src/js/discuss.js index 76350e0e34f..63885329caf 100644 --- a/addons/mail/static/src/js/discuss.js +++ b/addons/mail/static/src/js/discuss.js @@ -196,10 +196,6 @@ var Discuss = AbstractAction.extend(ControlPanelMixin, { /** * @override * @param {Object} [options] - * @param {boolean} [options.phantomjs=false] if set, rendering of non-empty - * thread will use 'block' display instead of 'flex', because phantomjs - * sucks with flexbox... TODO: remove this option when we no longer use - * phantomJS */ init: function (parent, action, options) { this._super.apply(this, arguments); @@ -223,9 +219,6 @@ var Discuss = AbstractAction.extend(ControlPanelMixin, { this._selectedMessage = null; this._throttledUpdateThreads = _.throttle( this._updateThreads.bind(this), 100, { leading: false }); - - // TODO: remove this option when we no longer use phantomJS - this._withPhantomJS = this.options.phantomjs || false; }, /** * @override @@ -459,7 +452,6 @@ var Discuss = AbstractAction.extend(ControlPanelMixin, { displayEmailIcons: false, displayReplyIcons: true, displayBottomThreadFreeSpace: true, - withPhantomJS: this._withPhantomJS, // TODO: remove this option when we no longer use phantomJS }; }, /** diff --git a/addons/mail/static/src/xml/thread.xml b/addons/mail/static/src/xml/thread.xml index 2ec6826b51b..55e6d250600 100644 --- a/addons/mail/static/src/xml/thread.xml +++ b/addons/mail/static/src/xml/thread.xml @@ -156,9 +156,6 @@ @param {boolean} [options.displayLoadMore=false] @param {string|integer} [options.messagesSeparatorPosition] 'top' or message ID, the separator is placed just after this message. - @param {boolean} [options.withPhantomJS=false] phantomJS is a mess with flexbox - so it may break some features like auto-fetch message on scroll. - TODO: remove this option when we no longer use phantomJS _____________ | | @@ -172,7 +169,7 @@ --> -
+
@@ -186,9 +183,6 @@ @param {Object} options @param {boolean} [options.displayBottomThreadFreeSpace=false] @param {boolean} [options.displayLoadMore=false] - @param {boolean} [options.withPhantomJS=false] phantomJS is a mess with flexbox - so it may break some features like auto-fetch message on scroll. - TODO: remove this option when we no longer use phantomJS _____________ | | @@ -201,7 +195,7 @@ DESC Order --> -
+
diff --git a/addons/mail/static/tests/discuss_tests.js b/addons/mail/static/tests/discuss_tests.js index 6182fd4a5ba..1ea0aab5ae9 100644 --- a/addons/mail/static/tests/discuss_tests.js +++ b/addons/mail/static/tests/discuss_tests.js @@ -412,8 +412,7 @@ QUnit.test('older messages are loaded on scroll', function (assert) { } return this._super.apply(this, arguments); }, - }, { phantomjs: true }) // TODO: remove this option when we no longer use phantomJS - .then(function (discuss) { + }).then(function (discuss) { assert.verifySteps(['message_fetch'], "should fetch messages once for needaction messages (Inbox)"); diff --git a/addons/mail/static/tests/helpers/test_utils.js b/addons/mail/static/tests/helpers/test_utils.js index 5e670701494..033866522e6 100644 --- a/addons/mail/static/tests/helpers/test_utils.js +++ b/addons/mail/static/tests/helpers/test_utils.js @@ -25,13 +25,9 @@ var Widget = require('web.Widget'); * This is async due to mail_manager/mail_service that needs to be ready. * * @param {Object} params - * @param {Object} options - * @param {boolean} options.phantomjs if set, rendering of non-empty thread will - * use 'block' display instead of 'flex', because phantomjs sucks with flexbox - * TODO: remove this option when we no longer use phantomJS * @return {$.Promise} resolved with the discuss widget */ -function createDiscuss(params, options) { +function createDiscuss(params) { var Parent = Widget.extend({ do_push_state: function () {}, }); @@ -41,7 +37,7 @@ function createDiscuss(params, options) { 'mail.message,false,search': '', }, })); - var discuss = new Discuss(parent, params, options); + var discuss = new Discuss(parent, params); discuss.set_cp_bus(new Widget()); var selector = params.debug ? 'body' : '#qunit-fixture'; var controlPanel = new ControlPanel(parent);