From 2d3287e6bf676a93e142147b6b517ce52b7fa9a0 Mon Sep 17 00:00:00 2001 From: Gilles Dubuc Date: Mon, 27 Jan 2014 18:12:09 +0100 Subject: [PATCH] Fixes bug where next/prev would exit fullscreen mode Next/prev caused an attach() call which in turn replaced the lightbox elements with themselves causing the browser to exit fullscreen mode because the full screened element was shortly removed from the DOM (to be replaced by the same content), Unfortunately this cannot be covered by a test case because the screen would have to truly go into fullscreen mode for the bug to happen. Having QUnit go fullscreen seems unreasonable both for local testing (annoying) and automated testing (could fail to really fo fullscreen because user confirmation is requested by the browser). Change-Id: I57a2668179d6749188d7b5a3efcd06216b7747a9 Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/139 --- resources/multilightbox/lightboxinterface.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/resources/multilightbox/lightboxinterface.js b/resources/multilightbox/lightboxinterface.js index 524e73452..e69dd66de 100644 --- a/resources/multilightbox/lightboxinterface.js +++ b/resources/multilightbox/lightboxinterface.js @@ -104,6 +104,12 @@ * @param {string} [parentId] parent id where we want to attach the UI. Mainly for testing. */ LIP.attach = function ( parentId ) { + // Re-appending the same content can have nasty side-effects + // Such as the browser leaving fullscreen mode if the fullscreened element is part of it + if ( this.currentlyAttached ) { + return; + } + var parent = $( parentId || document.body ); parent