From 09862cffecee0e709cd93874fd2e3be051a22455 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 23 Oct 2020 00:27:24 +0100 Subject: [PATCH] ext.uls.eventlogger: Remove more obsolete deferred complexity Follows-up c578db02 and ea671b1f, which I thought removed all code relating to the old async EventLogging method. I didn't notice this UI code at the time, because I only took a single pass over the code to find dead code. Now that that code is gone, it is clear that the UI code is also redundant. The mw.track() and logEvent() methods don't track the Beacon API's async fetches, which also isn't needed, since the loss of browser context upon navigation doesn't abort background beacons. The loading of the EL library itself is already ensured via a dependency so we already know there won't be an async fetch for that. What that leaves is some portion of older browsers in which a EventLogging falls back to 'new Image'. This is basically just IE 11 per , and for those some portion of events will have been lost since EventLogging removed support two+ years ago for tracking those fallback fetches via a Promise (because of the perf issues caused by what the removed code here was able to do). Change-Id: Idf4378f983b6ba0e755ebadb97aa6d87cf95f7a5 --- resources/js/ext.uls.common.js | 89 +++++++++++-------------- resources/js/ext.uls.displaysettings.js | 32 +++------ resources/js/ext.uls.eventlogger.js | 14 +--- resources/js/ext.uls.interface.js | 17 ++--- 4 files changed, 55 insertions(+), 97 deletions(-) diff --git a/resources/js/ext.uls.common.js b/resources/js/ext.uls.common.js index b0163b86..582a65f8 100644 --- a/resources/js/ext.uls.common.js +++ b/resources/js/ext.uls.common.js @@ -39,7 +39,7 @@ * @param {string} language Language code. */ mw.uls.changeLanguage = function ( language ) { - var deferred = new $.Deferred(); + var api = new mw.Api(); function changeLanguageAnon() { if ( mw.config.get( 'wgULSAnonCanChangeLanguage' ) ) { @@ -48,59 +48,50 @@ } } - deferred.done( function () { - var api = new mw.Api(); + // Track if event logging is enabled + mw.hook( 'mw.uls.interface.language.change' ).fire( language ); - if ( mw.user.isAnon() ) { - changeLanguageAnon(); - return; + if ( mw.user.isAnon() ) { + changeLanguageAnon(); + return; + } + + // TODO We can avoid doing this query if we know global preferences are not enabled + api.get( { + action: 'query', + meta: 'globalpreferences', + gprprop: 'preferences' + } ).then( function ( res ) { + // Check whether global preferences are in use. If they are not, `res.query` is + // an empty object. `res` will also contain warnings about unknown parameters. + try { + return !!res.query.globalpreferences.preferences.language; + } catch ( e ) { + return false; + } + } ).then( function ( hasGlobalPreference ) { + var apiModule; + + if ( hasGlobalPreference ) { + apiModule = 'globalpreferenceoverrides'; + mw.storage.set( 'uls-gp', '1' ); + } else { + apiModule = 'options'; + mw.storage.remove( 'uls-gp' ); } - // TODO We can avoid doing this query if we know global preferences are not enabled - api.get( { - action: 'query', - meta: 'globalpreferences', - gprprop: 'preferences' - } ).then( function ( res ) { - // Check whether global preferences are in use. If they are not, `res.query` is - // an empty object. `res` will also contain warnings about unknown parameters. - try { - return !!res.query.globalpreferences.preferences.language; - } catch ( e ) { - return false; - } - } ).then( function ( hasGlobalPreference ) { - var apiModule; - - if ( hasGlobalPreference ) { - apiModule = 'globalpreferenceoverrides'; - mw.storage.set( 'uls-gp', '1' ); - } else { - apiModule = 'options'; - mw.storage.remove( 'uls-gp' ); - } - - return api.postWithToken( 'csrf', { - action: apiModule, - optionname: 'language', - optionvalue: language - } ); - } ).done( function () { - location.reload(); - } ).fail( function () { - // Setting the option failed. Maybe the user has logged off. - // Continue like anonymous user and set cookie. - changeLanguageAnon(); + return api.postWithToken( 'csrf', { + action: apiModule, + optionname: 'language', + optionvalue: language } ); + } ).done( function () { + location.reload(); + } ).fail( function () { + // Setting the option failed. Maybe the user has logged off. + // Continue like anonymous user and set cookie. + changeLanguageAnon(); } ); - - mw.hook( 'mw.uls.interface.language.change' ).fire( language, deferred ); - - // Delay is zero if event logging is not enabled - setTimeout( function () { - deferred.resolve(); - }, mw.config.get( 'wgULSEventLogging' ) * 500 ); - }; mw.uls.setPreviousLanguages = function ( previousLanguages ) { diff --git a/resources/js/ext.uls.displaysettings.js b/resources/js/ext.uls.displaysettings.js index c68875a0..f756b0e8 100644 --- a/resources/js/ext.uls.displaysettings.js +++ b/resources/js/ext.uls.displaysettings.js @@ -207,30 +207,14 @@ new mw.Api().parse( $.i18n( 'ext-uls-display-settings-anon-log-in-cta' ) ) .done( function ( parsedCta ) { - var deferred = new $.Deferred(); - - $loginCta.html( parsedCta ); // The parsed CTA is HTML - $loginCta.find( 'a' ).on( 'click', function ( event ) { - event.preventDefault(); - // Because browsers navigate away when clicking a link, - // we are overriding the normal click behavior to allow - // the event be logged first - currently there is no - // local queue for events. Since the hook system does not - // allow returning values, we have this ugly hack - // for event logging to delay the page loading if event logging - // 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 () { - location.href = event.target.href; - } ); - - mw.hook( 'mw.uls.login.click' ).fire( deferred ); - - // Delay is zero if event logging is not enabled - setTimeout( function () { - deferred.resolve(); - }, mw.config.get( 'wgULSEventLogging' ) * 500 ); + // The parsed CTA is HTML + $loginCta.html( parsedCta ); + $loginCta.find( 'a' ).on( 'click', function () { + // If EventLogging is installed and enabled for ULS, give it a + // chance to log this event. There is no promise provided and in + // most browsers this will use the Beacon API in the background. + // In older browsers, this event will likely get lost. + mw.hook( 'mw.uls.login.click' ); } ); } ); diff --git a/resources/js/ext.uls.eventlogger.js b/resources/js/ext.uls.eventlogger.js index a5f4a0be..35ea6867 100644 --- a/resources/js/ext.uls.eventlogger.js +++ b/resources/js/ext.uls.eventlogger.js @@ -57,12 +57,9 @@ /** * Log language revert - * - * @param {jQuery.Deferred} deferred */ - function ulsLanguageRevert( deferred ) { + function ulsLanguageRevert() { log( { action: 'ui-lang-revert' } ); - deferred.resolve(); } /** @@ -97,12 +94,9 @@ /** * Log login link click in display settings. - * - * @param {jQuery.Deferred} deferred */ - function loginClick( deferred ) { + function loginClick() { log( { action: 'login-click' } ); - deferred.resolve(); } /** @@ -119,9 +113,8 @@ * Log interface language change * * @param {string} language language code - * @param {jQuery.Deferred} deferred */ - function interfaceLanguageChange( language, deferred ) { + function interfaceLanguageChange( language ) { var logParams = { action: 'language-change', context: 'interface', @@ -129,7 +122,6 @@ }; log( logParams ); - deferred.resolve(); } /** diff --git a/resources/js/ext.uls.interface.js b/resources/js/ext.uls.interface.js index b95e4205..057264df 100644 --- a/resources/js/ext.uls.interface.js +++ b/resources/js/ext.uls.interface.js @@ -192,21 +192,12 @@ dir: 'auto' } ) .on( 'click', function ( event ) { - var deferred = $.Deferred(); + // Track if event logging is enabled + mw.hook( 'mw.uls.language.revert' ).fire(); - event.preventDefault(); - deferred.done( function () { - mw.loader.using( [ 'ext.uls.common' ] ).then( function () { - mw.uls.changeLanguage( event.target.lang ); - } ); + mw.loader.using( [ 'ext.uls.common' ] ).then( function () { + mw.uls.changeLanguage( event.target.lang ); } ); - - mw.hook( 'mw.uls.language.revert' ).fire( deferred ); - - // Delay is zero if event logging is not enabled - setTimeout( function () { - deferred.resolve(); - }, mw.config.get( 'wgULSEventLogging' ) * 500 ); } ); if ( mw.storage.get( 'uls-gp' ) === '1' ) {