Catrope has uploaded a new change for review. https://gerrit.wikimedia.org/r/177107
Change subject: Reduce complexity around onbeforeunload handler ...................................................................... Reduce complexity around onbeforeunload handler We used to attempt to not break Firefox's bfcache, but this didn't really work very well, and it's not clear that avoiding bfcache breakage is even a good idea. It's more sensible and consistent to deliberately break bfcache while VE is active. On top of that, the Firefox people believe that our trick shouldn't even have worked to begin with: https://bugzilla.mozilla.org/show_bug.cgi?id=1102664 Because the onbeforeunload handler is removed when VE is deactivated, bfcache still works if you first click Read, then navigate away. Change-Id: Ie4fe9ea3a59a54ba462733aa5e42bfc0ed5b15eb --- M modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js 1 file changed, 10 insertions(+), 43 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor refs/changes/07/177107/1 diff --git a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js index c345961..9d3316d 100644 --- a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js +++ b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js @@ -33,7 +33,6 @@ this.toolbarSaveButton = null; this.saveDialog = null; this.onBeforeUnloadFallback = null; - this.onBeforeUnloadHandler = null; this.active = false; this.activating = false; this.deactivating = false; @@ -1554,13 +1553,7 @@ // Remember any already set on before unload handler this.onBeforeUnloadFallback = window.onbeforeunload; // Attach before unload handler - window.onbeforeunload = this.onBeforeUnloadHandler = this.onBeforeUnload.bind( this ); - // Attach page show handlers - if ( window.addEventListener ) { - window.addEventListener( 'pageshow', this.onPageShow.bind( this ), false ); - } else if ( window.attachEvent ) { - window.attachEvent( 'pageshow', this.onPageShow.bind( this ) ); - } + window.onbeforeunload = this.onBeforeUnload.bind( this ); }; /** @@ -1571,6 +1564,7 @@ ve.init.mw.ViewPageTarget.prototype.tearDownBeforeUnloadHandler = function () { // Restore whatever previous onbeforeload hook existed window.onbeforeunload = this.onBeforeUnloadFallback; + this.onBeforeUnloadFallback = null; }; /** @@ -1624,52 +1618,25 @@ }; /** - * Handle page show event. - * - * @method - */ -ve.init.mw.ViewPageTarget.prototype.onPageShow = function () { - // Re-add onbeforeunload handler - window.onbeforeunload = this.onBeforeUnloadHandler; -}; - -/** * Handle before unload event. * * @method */ ve.init.mw.ViewPageTarget.prototype.onBeforeUnload = function () { - var fallbackResult, - message, - onBeforeUnloadHandler = this.onBeforeUnloadHandler; + var fallbackResult; // Check if someone already set on onbeforeunload hook if ( this.onBeforeUnloadFallback ) { // Get the result of their onbeforeunload hook fallbackResult = this.onBeforeUnloadFallback(); - } - // Check if their onbeforeunload hook returned something - if ( fallbackResult !== undefined ) { - // Exit here, returning their message - message = fallbackResult; - } else { - // Override if submitting - if ( this.submitting ) { - return undefined; - } - // Check if there's been an edit - if ( this.surface && this.edited && mw.user.options.get( 'useeditwarning' ) ) { - // Return our message - message = ve.msg( 'visualeditor-viewpage-savewarning' ); + // If it returned something, exit here and return their message + if ( fallbackResult !== undefined ) { + return fallbackResult; } } - // Unset the onbeforeunload handler so we don't break page caching in Firefox - window.onbeforeunload = null; - if ( message !== undefined ) { - // ...but if the user chooses not to leave the page, we need to rebind it - setTimeout( function () { - window.onbeforeunload = onBeforeUnloadHandler; - } ); - return message; + // Check if there's been an edit + if ( this.surface && this.edited && !this.submitting && mw.user.options.get( 'useeditwarning' ) ) { + // Return our message + return ve.msg( 'visualeditor-viewpage-savewarning' ); } }; -- To view, visit https://gerrit.wikimedia.org/r/177107 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie4fe9ea3a59a54ba462733aa5e42bfc0ed5b15eb Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/VisualEditor Gerrit-Branch: master Gerrit-Owner: Catrope <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
