Santhosh has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/79452


Change subject: Followup I59dfcfb25c, language change work with out event 
logging
......................................................................

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 and 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 eventlogging, it is wrong.

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

Change-Id: I0b7d9d8b9d1d01b99422010596ebfa80b2589d04
---
M resources/js/ext.uls.displaysettings.js
M resources/js/ext.uls.eventlogger.js
M resources/js/ext.uls.init.js
M resources/js/ext.uls.interface.js
4 files changed, 20 insertions(+), 37 deletions(-)


  git pull 
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UniversalLanguageSelector 
refs/changes/52/79452/1

diff --git a/resources/js/ext.uls.displaysettings.js 
b/resources/js/ext.uls.displaysettings.js
index 8cffd85..d993a5f 100644
--- a/resources/js/ext.uls.displaysettings.js
+++ b/resources/js/ext.uls.displaysettings.js
@@ -167,9 +167,10 @@
                                                        // local queue for 
events. The timeout is there to make sure
                                                        // the user gets to the 
new page even if event logging is slow
                                                        // or fails.
-                                                       mw.hook( 
'mw.uls.login.click' ).fire( function () {
+                                                       mw.hook( 
'mw.uls.login.click' ).fire();
+                                                       window.settimeout( 
function () {
                                                                
window.location.href = event.target.href;
-                                                       } );
+                                                       }, 500 );
                                                } );
                                        } );
 
diff --git a/resources/js/ext.uls.eventlogger.js 
b/resources/js/ext.uls.eventlogger.js
index 3631e05..7fa11d1 100644
--- a/resources/js/ext.uls.eventlogger.js
+++ b/resources/js/ext.uls.eventlogger.js
@@ -48,8 +48,7 @@
                },
 
                /**
-                * Local wrapper for 'mw.eventLog.logEvent' which handles 
default params
-                * and ensures the correct schema is loaded.
+                * Local wrapper for 'mw.eventLog.logEvent'
                 *
                 * @param {Object} event Event action and optional fields
                 * @param {int} [timeout] Fail the request if it is not 
completed within
@@ -106,11 +105,9 @@
 
                /**
                 * Log language revert
-                *
-                * @param {Function} callback
                 */
-               ulsLanguageRevert: function ( callback ) {
-                       this.log( { action: 'ui-lang-revert' }, 500 ).always( 
callback );
+               ulsLanguageRevert: function () {
+                       this.log( { action: 'ui-lang-revert' } );
                },
 
                /**
@@ -128,17 +125,10 @@
                },
 
                /**
-                * Log login link click in display settings. Since this will 
navigate
-                * away from the current page, provide a callback option.
-                *
-                * 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
+                * Log login link click in display settings.
                 */
-               loginClick: function ( callback ) {
-                       this.log( { action: 'login-click' }, 500 ).always( 
callback );
+               loginClick: function () {
+                       this.log( { action: 'login-click' } );
                },
 
                /**
@@ -155,14 +145,13 @@
                 * Log interface language change
                 *
                 * @param {string} language language code
-                * @param {Function} callback
                 */
-               interfaceLanguageChange: function ( language, callback ) {
+               interfaceLanguageChange: function ( language ) {
                        this.log( {
                                action: 'language-change',
                                context: 'interface',
                                interfaceLanguage: language
-                       }, 500 ).always( callback );
+                       } );
                },
 
                /**
@@ -177,5 +166,5 @@
        };
 
        mw.uls = mw.uls || {};
-       mw.uls.eventlogger= new ULSEventLogger();
+       mw.uls.eventlogger = new ULSEventLogger();
 }( jQuery, mediaWiki ) );
diff --git a/resources/js/ext.uls.init.js b/resources/js/ext.uls.init.js
index 6bfe299..f9f87c4 100644
--- a/resources/js/ext.uls.init.js
+++ b/resources/js/ext.uls.init.js
@@ -46,12 +46,14 @@
        mw.uls.changeLanguage = function ( language ) {
                var uri = new mw.Uri( window.location.href );
 
-               mw.hook( 'mw.uls.interface.language.change' ).fire( language, 
function () {
+               mw.hook( 'mw.uls.interface.language.change' ).fire( language );
+               window.setTimeout( function () {
                        uri.extend( {
                                setlang: language
                        } );
                        window.location.href = uri.toString();
-               } );
+               }, 500 );
+
        };
 
        mw.uls.setPreviousLanguages = function ( previousLanguages ) {
@@ -70,18 +72,8 @@
                return $.parseJSON( previousLanguages ).slice( -5 );
        };
 
-       /**
-        * Returns the browser's user interface language or the system language.
-        * The caller should check the validity of the returned language code.
-        *
-        * @return {string} Language code or empty string.
-        */
        mw.uls.getBrowserLanguage = function () {
-               // language is the standard property.
-               // userLanguage is only for IE and returns system locale.
-               // Empty string is a fallback in case both are undefined
-               // to avoid runtime error with split().
-               return ( window.navigator.language || 
window.navigator.userLanguage || '' ).split( '-' )[0];
+               return ( window.navigator.language || 
window.navigator.userLanguage ).split( '-' )[0];
        };
 
        /*jshint camelcase:false*/
diff --git a/resources/js/ext.uls.interface.js 
b/resources/js/ext.uls.interface.js
index 28f74a1..4c42e8b 100644
--- a/resources/js/ext.uls.interface.js
+++ b/resources/js/ext.uls.interface.js
@@ -248,9 +248,10 @@
                        // there wont be multiple event handlers bound to same 
click.
                        $( 'a.uls-prevlang-link' ).on( 'click.ulstipsy', 
function ( event ) {
                                event.preventDefault();
-                               mw.hook( 'mw.uls.language.revert' ).fire( 
function () {
+                               mw.hook( 'mw.uls.language.revert' ).fire();
+                               window.setTimeout( function () {
                                        mw.uls.changeLanguage( 
event.target.lang );
-                               } );
+                               }, 500 );
                        } );
                        tipsyTimer = window.setTimeout( function () {
                                hideTipsy();

-- 
To view, visit https://gerrit.wikimedia.org/r/79452
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0b7d9d8b9d1d01b99422010596ebfa80b2589d04
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/UniversalLanguageSelector
Gerrit-Branch: master
Gerrit-Owner: Santhosh <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to