Fix regressions in ULS entry point setup

* Link in preferences is again opening ULS
* "personal" entry point now works even with compact languages
  preference disabled

I added lots of comments and refactored this code to give things
better names to make this code easier to understand and maintain
and hopefully reduce the amount of bugs in the future.

Bug: T282956
Bug: T286574
Change-Id: I9cd6776ea6664e33fd63c49dfa77c5f004fba799

includes/UniversalLanguageSelectorHooks.php

@@ -424,7 +424,7 @@ class UniversalLanguageSelectorHooks {
 			'section' => 'personal/i18n',
 			// We use this class to hide this from no-JS users
 			'cssclass' => 'uls-preferences-link-wrapper',
-			'default' => "<a id='uls-preferences-link' role='button' tabindex='0'>" .
+			'default' => "<a id='uls-preferences-link' class='uls-settings-trigger' role='button' tabindex='0'>" .
 				wfMessage( 'ext-uls-language-settings-preferences-link' )->escaped() . "</a>",
 		];
 

resources/js/ext.uls.interface.js

@@ -234,116 +234,108 @@
 		} );
 	}
 
-	function initInterface() {
+	function initSecondaryEntryPoints() {
-		var $pLang,
+		$( '.uls-settings-trigger' ).one( 'click', function ( e ) {
-			clickHandler,
+			e.preventDefault();
-			// T273928: No change to the heading should be made in modern Vector when the language
+			mw.loader.using( languageSettingsModules, function () {
-			// button is present
+				$( e.target ).languagesettings();
-			isButton = isUsingStandaloneLanguageButton(),
+				$( e.target ).trigger( 'click' );
-			$trigger = $( '.uls-trigger' ),
+			} );
-			anonMode = ( mw.user.isAnon() &&
+		} );
-				!mw.config.get( 'wgULSAnonCanChangeLanguage' ) ),
+	}
-			configPosition = mw.config.get( 'wgULSPosition' );
 
-		if ( !mw.config.get( 'wgULSisCompactLinksEnabled' ) ) {
+	function initInterlanguageEntryPoint() {
-			// The wgULSisCompactLinksEnabled flag when disabled will not render a language button to the page
+		var $pLang = $( '#p-lang' );
-			// Skins can control where the button is placed, by adding an element with mw-interlanguage-selector to the page,
+
-			// the display of which is not impacted by this flag. To signal to these skins that the language button should be
+		var $trigger = $( '<button>' )
-			// disabled, the class is removed.
+			.addClass( 'uls-settings-trigger' )
-			$( '.mw-interlanguage-selector' ).removeClass( 'mw-interlanguage-selector' );
+			.prop( 'title', mw.msg( 'ext-uls-select-language-settings-icon-tooltip' ) );
-			return;
+
+		// Append ULS cog to interlanguage section header in the sidebar
+		$pLang.prepend( $trigger );
+
+		// Replace the title of the interlanguage links area from "In other languages" to
+		// "Languages" if there are no language links. TODO: Remove this feature?
+		if ( !$pLang.find( 'div ul' ).children().length && isUsingStandaloneLanguageButton ) {
+			$pLang.find( 'h3' ).text( mw.msg( 'uls-plang-title-languages' ) );
 		}
 
-		if ( configPosition === 'interlanguage' ) {
+		var clickHandler = function ( e ) {
-			// TODO: Refactor this block
+			var languagesettings = $trigger.data( 'languagesettings' ),
-			// The interlanguage links section.
+				languageSettingsOptions;
-			$pLang = $( '#p-lang' );
-			// Add an element near the interlanguage links header
-			$trigger = $( '<button>' )
-				.addClass( 'uls-settings-trigger' );
-			// Append ULS cog to languages section.
-			$pLang.prepend( $trigger );
-			// Take care of any other elements with this class.
-			$trigger = $( '.uls-settings-trigger' );
 
-			if ( !$pLang.find( 'div ul' ).children().length && isButton ) {
+			if ( languagesettings ) {
-				// Replace the title of the interlanguage links area
+				if ( !languagesettings.shown ) {
-				// if there are no interlanguage links
+					mw.hook( 'mw.uls.settings.open' ).fire( 'interlanguage' );
-				$pLang.find( 'h3' )
-					.text( mw.msg( 'uls-plang-title-languages' ) );
-			}
-
-			$trigger.attr( {
-				title: mw.msg( 'ext-uls-select-language-settings-icon-tooltip' )
-			} );
-
-			clickHandler = function ( e, eventParams ) {
-				var languagesettings = $trigger.data( 'languagesettings' ),
-					languageSettingsOptions;
-
-				if ( languagesettings ) {
-					if ( !languagesettings.shown ) {
-						mw.hook( 'mw.uls.settings.open' ).fire( eventParams && eventParams.source || 'interlanguage' );
-					}
-
-					return;
 				}
 
-				// Initialize the Language settings window
+				return;
-				languageSettingsOptions = {
+			}
-					defaultModule: 'display',
-					onPosition: function () {
-						var caretRadius, top, left,
-							ulsTriggerHeight = this.$element.height(),
-							ulsTriggerWidth = this.$element[ 0 ].offsetWidth,
-							ulsTriggerOffset = this.$element.offset();
 
-						// Same as border width in mixins.less, or near enough
+			// Initialize the Language settings window
-						caretRadius = 12;
+			languageSettingsOptions = {
+				defaultModule: 'display',
+				onPosition: function () {
+					var caretRadius, top, left,
+						ulsTriggerHeight = this.$element.height(),
+						ulsTriggerWidth = this.$element[ 0 ].offsetWidth,
+						ulsTriggerOffset = this.$element.offset();
 
-						if ( ulsTriggerOffset.left > $( window ).width() / 2 ) {
+					// Same as border width in mixins.less, or near enough
-							left = ulsTriggerOffset.left - this.$window.width() - caretRadius;
+					caretRadius = 12;
-							this.$window.removeClass( 'selector-left' ).addClass( 'selector-right' );
-						} else {
-							left = ulsTriggerOffset.left + ulsTriggerWidth + caretRadius;
-							this.$window.removeClass( 'selector-right' ).addClass( 'selector-left' );
-						}
 
-						// The top of the dialog is aligned in relation to
+					if ( ulsTriggerOffset.left > $( window ).width() / 2 ) {
-						// the middle of the trigger, so that middle of the
+						left = ulsTriggerOffset.left - this.$window.width() - caretRadius;
-						// caret aligns with it. 16 is trigger icon height in pixels
+						this.$window.removeClass( 'selector-left' ).addClass( 'selector-right' );
-						top = ulsTriggerOffset.top +
+					} else {
-							( ulsTriggerHeight / 2 ) -
+						left = ulsTriggerOffset.left + ulsTriggerWidth + caretRadius;
-							( caretRadius + 16 );
+						this.$window.removeClass( 'selector-right' ).addClass( 'selector-left' );
-
-						return { top: top, left: left };
-					},
-					onVisible: function () {
-						this.$window.addClass( 'callout' );
 					}
-				};
 
-				mw.loader.using( languageSettingsModules, function () {
+					// The top of the dialog is aligned in relation to
-					$trigger.languagesettings( languageSettingsOptions ).trigger( 'click' );
+					// the middle of the trigger, so that middle of the
-				} );
+					// caret aligns with it. 16 is trigger icon height in pixels
+					top = ulsTriggerOffset.top +
+						( ulsTriggerHeight / 2 ) -
+						( caretRadius + 16 );
 
-				e.stopPropagation();
+					return { top: top, left: left };
+				},
+				onVisible: function () {
+					this.$window.addClass( 'callout' );
+				}
 			};
-		} else if ( anonMode ) {
+
-			clickHandler = function ( e, eventParams ) {
+			mw.loader.using( languageSettingsModules, function () {
+				$trigger.languagesettings( languageSettingsOptions ).trigger( 'click' );
+			} );
+
+			e.stopPropagation();
+		};
+
+		$trigger.on( 'click', clickHandler );
+	}
+
+	function initPersonalEntryPoint() {
+		var $trigger = $( '.uls-trigger' );
+		var clickHandler;
+
+		var anonMode = ( mw.user.isAnon() && !mw.config.get( 'wgULSAnonCanChangeLanguage' ) );
+		if ( anonMode ) {
+			clickHandler = function ( e ) {
 				var languagesettings = $trigger.data( 'languagesettings' );
 
 				e.preventDefault();
 
 				if ( languagesettings ) {
 					if ( !languagesettings.shown ) {
-						mw.hook( 'mw.uls.settings.open' ).fire( eventParams && eventParams.source || 'personal' );
+						mw.hook( 'mw.uls.settings.open' ).fire( 'personal' );
 					}
 				} else {
 					mw.loader.using( languageSettingsModules, function () {
 						$trigger.languagesettings();
 
-						$trigger.trigger( 'click', eventParams );
+						$trigger.trigger( 'click' );
 					} );
 				}
 			};
@@ -355,7 +347,7 @@
 
 				if ( uls ) {
 					if ( !uls.shown ) {
-						mw.hook( 'mw.uls.settings.open' ).fire( eventParams && eventParams.source || 'personal' );
+						mw.hook( 'mw.uls.settings.open' ).fire( 'personal' );
 					}
 				} else {
 					mw.loader.using( 'ext.uls.mediawiki', function () {
@@ -390,24 +382,9 @@
 		}
 
 		$trigger.on( 'click', clickHandler );
-
-		// Bind language settings to preferences page link
-		$( '#uls-preferences-link' )
-			.on( 'click keypress', function ( e ) {
-				if (
-					e.type === 'click' ||
-					e.type === 'keypress' && e.which === 13
-				) {
-					$trigger.trigger( 'click', {
-						source: 'preferences'
-					} );
-				}
-
-				return false;
-			} );
 	}
 
-	function initTooltip() {
+	function initLanguageChangeUndoTooltip() {
 		var previousLanguage, currentLanguage, previousAutonym, currentAutonym;
 
 		if ( !userCanChangeLanguage() ) {
@@ -511,10 +488,51 @@
 	}
 
 	function init() {
-		initInterface();
+		initLanguageChangeUndoTooltip();
-		initTooltip();
 		initIme();
-		initContentLanguageSelectorClickHandler();
+
+		// There are three basic components of ULS interface:
+		// - language selection for interface
+		// - language selection for content
+		// - settings view (access to language selection for interface, fonts, input methods)
+		//
+		// These can be combined in different ways:
+		// - Vector skin (recently) has an omni selector that has content language selection as
+		//   primary action with access to the settings view. It is on top right corner (LTR) of
+		//   the page content area. It may not be present on all pages.
+		// - Compact language links provides access to content language selection only and it is in
+		//   the interlanguage section of the sidebar. This is in addition to one of the main entry
+		//   points below.
+		// - Personal entry point appears at the top of the page. It provides quick access to the
+		//   interface language selection with access to the settings view, except if user is not
+		//   logged in and not allowed to change a language. In this case it defaults to settings
+		//   view without language selection.
+		// - Interlanguage entry point (a cog) appears in the interlanguage section in the sidebar.
+		//   It defaults to the settings view.
+		//
+		// The three main entry points (omni selector, personal, interlanguage) are mutually
+		// exclusive. There may be secondary entry points anywhere on the page using the
+		// uls-settings-trigger class.
+
+		// First init secondary to avoid initing the interlanguage entry point multiple times
+		initSecondaryEntryPoints();
+		var position = mw.config.get( 'wgULSPosition' );
+		if ( position === 'interlanguage' ) {
+			initInterlanguageEntryPoint();
+		} else {
+			initPersonalEntryPoint();
+		}
+
+		var compact = mw.config.get( 'wgULSisCompactLinksEnabled' );
+		// The scope of the compact language links user preference has been expanded to also
+		// determine whether to show the omni box or not. Compact language links is already not
+		// loaded server side, so this is only relevant for the omnibox.
+		if ( compact ) {
+			// Init compact languages OR omni selector using the mw-interlanguage-selector class
+			initContentLanguageSelectorClickHandler();
+		} else {
+			$( '.mw-interlanguage-selector' ).removeClass( 'mw-interlanguage-selector' );
+		}
 	}
 
 	// Early execute of init