From ca1364928c85b8bcdc5048ba05356b08fdb826fc Mon Sep 17 00:00:00 2001 From: Matthias Mullie Date: Tue, 7 Jan 2020 09:55:09 +0100 Subject: [PATCH] Move mmv-close event trigger higher up in lightboxinterface/unattach This avoids a bug with Firefox's automatic scroll restoration logic, which seems to always jump to the top of the page upon closing MMV. Bug: T229484 Change-Id: Ia4261ce268df4d8fb07b4813a6ae72ad88728e91 --- resources/mmv/mmv.lightboxinterface.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/resources/mmv/mmv.lightboxinterface.js b/resources/mmv/mmv.lightboxinterface.js index 935800e2e..954c5b202 100644 --- a/resources/mmv/mmv.lightboxinterface.js +++ b/resources/mmv/mmv.lightboxinterface.js @@ -253,6 +253,14 @@ LIP.unattach = function () { mw.mmv.actionLogger.log( 'close' ); + // We trigger this event on the document because unattach() can run + // when the interface is unattached + // We're calling this before cleaning up (below) the DOM, as that + // appears to have an impact on automatic scroll restoration (which + // might happen as a result of this being closed) in FF + $( document ).trigger( $.Event( 'mmv-close' ) ) + .off( 'jq-fullscreen-change.lip' ); + // Has to happen first so that the scroller can freeze with visible elements this.panel.unattach(); @@ -284,11 +292,6 @@ prev: [ 'emit', 'prev' ] } ); - // We trigger this event on the document because unattach() can run - // when the interface is unattached - $( document ).trigger( $.Event( 'mmv-close' ) ) - .off( 'jq-fullscreen-change.lip' ); - this.attached = false; };