From c2d368b39cb9515f8c83101ce59c65dfcefc6e49 Mon Sep 17 00:00:00 2001 From: tsm-odoo Date: Fri, 23 Sep 2022 10:41:32 +0000 Subject: [PATCH] [IMP] bus: use 'pagehide' instead of 'unload' event The original goal here is to remove the warning of the google page speed but it actually is really better to use 'pagehide' as it is fired more reliably than the 'unload' event and does not prevent Chrome and Firefox to use the bfcache (browser optimization that enables instant back and forward navigation). The persisted property of the pagehide event is checked before disconnecting from the worker. As for the presence/multi_tab case, it was chosen to also consider page freezing as it seems more accurate that what was done before (the main tab could be throttled/frozen which was an issue). Some resources to explain why this is better: - https://web.dev/bfcache/#never-use-the-unload-event - https://developer.mozilla.org/en-US/docs/Web/API/Window/pagehide_event closes odoo/odoo#101309 X-original-commit: e043891d5074ca04ee1bbed421ae5efe5f2f8b88 Signed-off-by: Quentin Smetz (qsm) Co-authored-by: qsm-odoo --- addons/bus/static/src/multi_tab_service.js | 4 ++-- addons/bus/static/src/services/bus_service.js | 9 +++++++-- addons/bus/static/src/services/presence_service.js | 2 +- addons/bus/static/tests/bus_tests.js | 8 ++++---- addons/bus/static/tests/multi_tab_service_tests.js | 14 +++++++------- 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/addons/bus/static/src/multi_tab_service.js b/addons/bus/static/src/multi_tab_service.js index 080b626a2b1..47cfa2ec5b4 100644 --- a/addons/bus/static/src/multi_tab_service.js +++ b/addons/bus/static/src/multi_tab_service.js @@ -147,7 +147,7 @@ export const multiTabService = { } } - function onUnload() { + function onPagehide() { clearTimeout(heartbeatTimeout); const lastPresenceByTab = getItemFromStorage('lastPresenceByTab', {}); delete lastPresenceByTab[tabId]; @@ -161,7 +161,7 @@ export const multiTabService = { } } - browser.addEventListener('unload', onUnload); + browser.addEventListener('pagehide', onPagehide); browser.addEventListener('storage', onStorage); // REGISTER THIS TAB diff --git a/addons/bus/static/src/services/bus_service.js b/addons/bus/static/src/services/bus_service.js index 581cad4bfec..c47fce81d09 100644 --- a/addons/bus/static/src/services/bus_service.js +++ b/addons/bus/static/src/services/bus_service.js @@ -92,8 +92,13 @@ export const busService = { debug: odoo.debug, lastNotificationId: multiTab.getSharedValue('last_notification_id', 0), }); - browser.addEventListener('unload', () => send('leave')); - + browser.addEventListener('pagehide', ({ persisted }) => { + if (!persisted) { + // Page is gonna be unloaded, disconnect this client + // from the worker. + send('leave'); + } + }); return { addEventListener: bus.addEventListener.bind(bus), diff --git a/addons/bus/static/src/services/presence_service.js b/addons/bus/static/src/services/presence_service.js index 200a8f538fd..7370912dcdf 100644 --- a/addons/bus/static/src/services/presence_service.js +++ b/addons/bus/static/src/services/presence_service.js @@ -45,7 +45,7 @@ export const presenceService = { browser.addEventListener('storage', onStorage); browser.addEventListener('focus', () => onFocusChange(true)); browser.addEventListener('blur', () => onFocusChange(false)); - browser.addEventListener('unload', () => onFocusChange(false)); + browser.addEventListener('pagehide', () => onFocusChange(false)); browser.addEventListener('click', onPresence); browser.addEventListener('keydown', onPresence); diff --git a/addons/bus/static/tests/bus_tests.js b/addons/bus/static/tests/bus_tests.js index 70e58bfb9a1..c9c1e874d39 100644 --- a/addons/bus/static/tests/bus_tests.js +++ b/addons/bus/static/tests/bus_tests.js @@ -146,7 +146,7 @@ QUnit.module('Bus', { ]); }); - QUnit.test('second tab still receives notifications after main unload', async function (assert) { + QUnit.test('second tab still receives notifications after main pagehide', async function (assert) { assert.expect(4); const pyEnv = await startServer(); @@ -158,10 +158,10 @@ QUnit.module('Bus', { mainEnv.services['bus_service'].addChannel('lambda'); // second env - // prevent second tab from receiving unload event. + // prevent second tab from receiving pagehide event. patchWithCleanup(browser, { addEventListener(eventName, callback) { - if (eventName === 'unload') { + if (eventName === 'pagehide') { return; } this._super(eventName, callback); @@ -176,7 +176,7 @@ QUnit.module('Bus', { await nextTick(); // simulate unloading main - window.dispatchEvent(new Event('unload')); + window.dispatchEvent(new Event('pagehide')); await nextTick(); pyEnv['bus.bus']._sendone('lambda', 'notifType', 'gamma'); diff --git a/addons/bus/static/tests/multi_tab_service_tests.js b/addons/bus/static/tests/multi_tab_service_tests.js index 5cae1e03aa3..ac8d924e1ad 100644 --- a/addons/bus/static/tests/multi_tab_service_tests.js +++ b/addons/bus/static/tests/multi_tab_service_tests.js @@ -10,7 +10,7 @@ import { patchWithCleanup, nextTick } from '@web/../tests/helpers/utils'; QUnit.module('bus', function () { QUnit.module('multi_tab_service_tests.js'); - QUnit.test('multi tab service elects new master on unload', async function (assert) { + QUnit.test('multi tab service elects new master on pagehide', async function (assert) { assert.expect(5); registry.category('services').add('multi_tab', multiTabService); @@ -18,10 +18,10 @@ QUnit.module('bus', function () { const firstTabEnv = await makeTestEnv(); assert.ok(firstTabEnv.services['multi_tab'].isOnMainTab(), 'only tab should be the main one'); - // prevent second tab from receiving unload event. + // prevent second tab from receiving pagehide event. patchWithCleanup(browser, { addEventListener(eventName, callback) { - if (eventName === 'unload') { + if (eventName === 'pagehide') { return; } this._super(eventName, callback); @@ -34,7 +34,7 @@ QUnit.module('bus', function () { secondTabEnv.services["multi_tab"].bus.addEventListener("no_longer_main_tab", () => assert.step("tab2 no_longer_main_tab") ); - window.dispatchEvent(new Event('unload')); + window.dispatchEvent(new Event('pagehide')); // Let the multi tab elect a new main. await nextTick(); @@ -87,10 +87,10 @@ QUnit.module('bus', function () { registry.category('services').add('multi_tab', multiTabService); await makeTestEnv(); - // prevent second tab from receiving unload event. + // prevent second tab from receiving pagehide event. patchWithCleanup(browser, { addEventListener(eventName, callback) { - if (eventName === 'unload') { + if (eventName === 'pagehide') { return; } this._super(eventName, callback); @@ -98,7 +98,7 @@ QUnit.module('bus', function () { }); const secondTabEnv = await makeTestEnv(); secondTabEnv.services['multi_tab'].bus.addEventListener('become_main_tab', () => assert.step('become_main_tab')); - window.dispatchEvent(new Event('unload')); + window.dispatchEvent(new Event('pagehide')); // Let the multi tab elect a new main. await nextTick();