Followup I59dfcfb25c, language change work with out event logging

In I59dfcfb25c, for logging events when page is navigating away, we
used callbacks with mw.hook. That is wrong approach. If event logging
is disabled those callbacks will never called: it broke language change
and all use cases which navigates away from current page.

Event logging should not interfere with any ULS functionality. If ULS
functionality depends on callbacks from event logging, it is wrong.

In this patch, we give a small time window to make sure event logging is
fired, but we won't wait for its success or failure.

If eventlogging is disabled, this time window does not exist.

Change-Id: I0b7d9d8b9d1d01b99422010596ebfa80b2589d04

resources/js/ext.uls.displaysettings.js

@@ -158,18 +158,30 @@
 
 				new mw.Api().parse( $.i18n( 'ext-uls-display-settings-anon-log-in-cta' ) )
 					.done( function ( parsedCta ) {
+						var deferred = new $.Deferred();
+
 						$loginCta.html( parsedCta );
 						$loginCta.find( 'a' ).click( function ( event ) {
 							event.preventDefault();
 							// Because browsers navigate away when clicking a link,
 							// we are are overriding the normal click behavior to
 							// allow the event be logged first - currently there is no
-							// local queue for events. The timeout is there to make sure
+							// local queue for events. Since the hook system does not
-							// the user gets to the new page even if event logging is slow
+							// allow returning values, we have this ugly event logging
-							// or fails.
+							// specific hack to delay the page load if event logging
-							mw.hook( 'mw.uls.login.click' ).fire( function () {
+							// is enabled. The promise is passed to the hook, so that
+							// if event logging is enabled, in can resole the promise
+							// immediately to avoid extra delays.
+							deferred.done( function () {
 								window.location.href = event.target.href;
 							} );
+
+							mw.hook( 'mw.uls.login.click' ).fire( deferred );
+
+							// Delay is zero if event logging is not enabled
+							window.setTimeout( function () {
+								deferred.resolve();
+							}, mw.config.get( 'wgULSEventLogging' ) * 500 );
 						} );
 					} );
 

resources/js/ext.uls.eventlogger.js

@@ -48,21 +48,16 @@
 		},
 
 		/**
-		 * Local wrapper for 'mw.eventLog.logEvent' which handles default params
+		 * Local wrapper for 'mw.eventLog.logEvent'
-		 * and ensures the correct schema is loaded.
 		 *
 		 * @param {Object} event Event action and optional fields
-		 * @param {int} [timeout] Fail the request if it is not completed within
-		 *   the specified timeout. Can be use for example to log actions that
-		 *   cause the browser to navigate to other pages.
 		 * @return {jQuery.Promise} jQuery Promise object for the logging call
 		 */
-		log: function ( event, timeout ) {
+		log: function ( event ) {
 			// We need to create our own deferred for two reasons:
 			//  - logEvent might not be executed immediately
 			//  - we cannot reject a promise returned by it
-			// So we proxy the original promises status updates
+			// So we proxy the original promises status updates.
-			// and register our timeout if requested (for links).
 			var deferred = $.Deferred();
 
 			this.logEventQueue.add( function () {
@@ -71,10 +66,6 @@
 					.fail( deferred.reject );
 			} );
 
-			if ( timeout !== undefined ) {
-				window.setTimeout( deferred.reject, timeout );
-			}
-
 			return deferred.promise();
 		},
 
@@ -110,11 +101,10 @@
 
 		/**
 		 * Log language revert
-		 *
+		 * @param {jQuery.Deferred} deferred
-		 * @param {Function} callback
 		 */
-		ulsLanguageRevert: function ( callback ) {
+		ulsLanguageRevert: function ( deferred ) {
-			this.log( { action: 'ui-lang-revert' }, 500 ).always( callback );
+			this.log( { action: 'ui-lang-revert' } ).always( deferred.resolve() );
 		},
 
 		/**
@@ -132,17 +122,11 @@
 		},
 
 		/**
-		 * Log login link click in display settings. Since this will navigate
+		 * Log login link click in display settings.
-		 * away from the current page, provide a callback option.
+		 * @param {jQuery.Deferred} deferred
-		 *
-		 * If page is navigating away, event logging will fail because it is not
-		 * yet completed. With the callback, the navigation can be executed in
-		 * callback. The waiting for callback is not indefinite, but with a timeout.
-		 *
-		 * @param {Function} callback
 		 */
-		loginClick: function ( callback ) {
+		loginClick: function ( deferred ) {
-			this.log( { action: 'login-click' }, 500 ).always( callback );
+			this.log( { action: 'login-click' } ).always( deferred.resolve );
 		},
 
 		/**
@@ -159,14 +143,14 @@
 		 * Log interface language change
 		 *
 		 * @param {string} language language code
-		 * @param {Function} callback
+		 * @param {jQuery.Deferred} deferred
 		 */
-		interfaceLanguageChange: function ( language, callback ) {
+		interfaceLanguageChange: function ( language, deferred ) {
 			this.log( {
 				action: 'language-change',
 				context: 'interface',
 				interfaceLanguage: language
-			}, 500 ).always( callback );
+			} ).always( deferred.resolve );
 		},
 
 		/**

resources/js/ext.uls.init.js

@@ -44,14 +44,23 @@
 	 * @param {string} language Language code.
 	 */
 	mw.uls.changeLanguage = function ( language ) {
-		var uri = new mw.Uri( window.location.href );
+		var uri = new mw.Uri( window.location.href ),
+			deferred = new $.Deferred();
 
-		mw.hook( 'mw.uls.interface.language.change' ).fire( language, function () {
+		deferred.done( function () {
 			uri.extend( {
 				setlang: language
 			} );
 			window.location.href = uri.toString();
 		} );
+
+		mw.hook( 'mw.uls.interface.language.change' ).fire( language, deferred );
+
+		// Delay is zero if event logging is not enabled
+		window.setTimeout( function () {
+			deferred.resolve();
+		}, mw.config.get( 'wgULSEventLogging' ) * 500  );
+
 	};
 
 	mw.uls.setPreviousLanguages = function ( previousLanguages ) {

resources/js/ext.uls.interface.js

@@ -247,10 +247,19 @@
 			// It looks like the tipsy is always created from scratch so that
 			// there wont be multiple event handlers bound to same click.
 			$( 'a.uls-prevlang-link' ).on( 'click.ulstipsy', function ( event ) {
+				var deferred = $.Deferred();
+
 				event.preventDefault();
-				mw.hook( 'mw.uls.language.revert' ).fire( function () {
+				deferred.done( function () {
 					mw.uls.changeLanguage( event.target.lang );
 				} );
+
+				mw.hook( 'mw.uls.language.revert' ).fire( deferred );
+
+				// Delay is zero if event logging is not enabled
+				window.setTimeout( function () {
+					deferred.resolve();
+				}, mw.config.get( 'wgULSEventLogging' ) * 500 );
 			} );
 			tipsyTimer = window.setTimeout( function () {
 				hideTipsy();