Page MenuHomePhabricator

Jumbo size text and icons in personal menu intermittently in Vector skin
Closed, ResolvedPublic

Description

Noticed this while browsing mediawiki.org:

capture.png (443×2 px, 91 KB)

QA Results - Prod

ACStatusDetails
1T305262#7847024

Event Timeline

HTML:

<nav id="p-personal" class="mw-portlet mw-portlet-personal vector-user-menu-legacy vector-menu" aria-labelledby="p-personal-label" role="navigation">
	<label id="p-personal-label" aria-label="" class="vector-menu-heading">
		<span class="vector-menu-heading-label">Personal tools</span>
	</label>
	<div class="vector-menu-content">
		
		<ul class="vector-menu-content-list"><li id="pt-uls" class="mw-list-item active"><a class="user-links-collapsible-item" href="#"><span>English</span></a></li><li id="pt-userpage" class="mw-list-item"><a class="user-links-collapsible-item" href="/wiki/User:Krinkle" dir="auto" title="Your user page [⌃⌥.]" accesskey="."><span>Krinkle</span></a></li><li id="pt-notifications-alert" class="mw-list-item"><a class="mw-echo-notifications-badge mw-echo-notification-badge-nojs oo-ui-icon-bell" href="/wiki/Special:Notifications" data-counter-num="1" data-counter-text="1" title="Your alerts"><span>Alert (1)</span></a></li><li id="pt-notifications-notice" class="mw-list-item"><a class="mw-echo-notifications-badge mw-echo-notification-badge-nojs oo-ui-icon-tray mw-echo-notifications-badge-all-read" href="/wiki/Special:Notifications" data-counter-num="0" data-counter-text="0" title="Your notices"><span>Notices (0)</span></a></li><li id="pt-mytalk" class="mw-list-item"><a href="/wiki/User_talk:Krinkle" title="Your talk page [⌃⌥n]" accesskey="n"><span>Talk</span></a></li><li id="pt-preferences" class="mw-list-item"><a class="mw-ui-icon mw-ui-icon-before mw-ui-icon-settings mw-ui-icon-wikimedia-settings" href="/wiki/Special:Preferences" title="Your preferences"><span>Preferences</span></a></li><li id="pt-betafeatures" class="mw-list-item"><a class="mw-ui-icon mw-ui-icon-before mw-ui-icon-labFlask mw-ui-icon-wikimedia-labFlask" href="/wiki/Special:Preferences#mw-prefsection-betafeatures" title="Beta features"><span>Beta</span></a></li><li id="pt-watchlist" class="mw-list-item"><a class="user-links-collapsible-item" href="/wiki/Special:Watchlist" title="A list of pages you are monitoring for changes [⌃⌥l]" accesskey="l"><span>Watchlist</span></a></li><li id="pt-mycontris" class="mw-list-item"><a class="mw-ui-icon mw-ui-icon-before mw-ui-icon-userContributions mw-ui-icon-wikimedia-userContributions" href="/wiki/Special:Contributions/Krinkle" title="A list of your contributions [⌃⌥y]" accesskey="y"><span>Contributions</span></a></li></ul>
		
	</div>
</nav>

I'm seeing this too, as if it's T302627, but with different symptoms.

The icons are added by the hook in https://gerrit.wikimedia.org/g/mediawiki/skins/Vector/+/475508647466ea1c67d3e0d378ca3b4a76668e84/includes/Hooks.php#628

When the skin is not vector-2022 as is the case here it
checks the Constants::FEATURE_LATEST_SKIN feature
(https://gerrit.wikimedia.org/g/mediawiki/skins/Vector/+/475508647466ea1c67d3e0d378ca3b4a76668e84/includes/FeatureManagement/Requirements/LatestSkinVersionRequirement.php#81)

public function isMet(): bool {
		$useSkin = $this->request->getVal( 'useskin' );
		$user = $this->user;
		if ( !$useSkin && $user->isSafeToLoad() ) {
			$useSkin = $this->userOptionsLookup->getOption(
				$user,
				Constants::PREF_KEY_SKIN
			);
		}
		return $useSkin === Constants::SKIN_NAME_MODERN;
	}

Since useskin parameter is not used here if User was not safe to load then the feature would report as disabled thus isLegacy would report itself as true. This means that code is looking up the skin locally which is reporting back that it's vector-2022

My guess is this check is overlooking a global preference both of you have set. The local preference would be vector-2022 (the default).

The fix for T305232 will also fix this if this is indeed the problem.

In addition to partially corrupt HTML output (as indicated by the icons), there seems to be CSS involved as well. Note how the "Discussion" tab has the bright New Vector red for it text colour.

The presence of $user->isSafeToLoad() suggests the code known to be running running incorrectly. Outside of very internal core setup code, nothing should worry about isSafeToLoad.

Why does this check exist here? Why is it returning something different depending on how far into the process you call it? That seems designed to cause this kind of corruption.

Perhaps throw an exception from there (during development) instead of returning known incorrect data, then fix the caller(s) that are happening from inappropiate hooks or other early pre-init contexts, and then turn it into a real fatal for production once we're confidence there are no broken calls left.

fwiw, I'm seeing this on officewiki as well. (A fishbowl wiki without CentralAuth and GlobalPrefs.)

Change 776226 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Drop the LatestSkinVersionRequirement

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

Change 776226 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Drop the LatestSkinVersionRequirement

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

Edtadros added a subscriber: Edtadros.

Test Result - Prod

Status: ✅ PASS
Environment: mediawiki
OS: macOS Monterey
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

✅ AC1: Menu text should be the same size without bet, preferences, or contributions icons.

Screen Shot 2022-04-11 at 6.41.11 PM.png (165×1 px, 58 KB)

Looks good, resolving