Page MenuHomePhabricator

Users who have selected legacy Vector, seeing Vector 2022 on certain page views
Closed, ResolvedPublic

Description

In T302627, @Krinkle helpfully added a script to French Wikipedia for logged in users that checks the skin preference and checks certain elements in the page (the Vector 2022 hamburger icon, the sidebar action "switch to old look" and the user avatar icon in the top right corner). For users were the skin was set to legacy Vector, and Vector 2022 elements were present in the page, an event was logged that was plotted into this graph:
https://grafana.wikimedia.org/d/OHBhRhy7k/vector-bug-t302627

The code suggests that at a rate of 1-6 page views a minute, a user of legacy Vector is receiving something that looks like modern Vector.

In T302627#7846955, @Krinkle poked concern at the "numerous unsafe service classes that Vector registers" as being the cause, however what is strange here, is that the hamburger icon is only rendered inside includes/templates/skin.mustache which can only be rendered if SkinVector22 is the skin class (this is set inside skin.json) and as far as I (@Jdlrobson) can see has nothing to do with the services.

We have had various reports (T288113, T300278, T303265, T305232, T305262) of strange rendering issues inside Vector. While some of them were almost certain related to the skin splitting of Vector, issues clearly still remain and questions unanswered.

Questions to answer

  • How is it possible that a user preference can be set to 'vector' but the user sees modern Vector?
  • Walkthrough feature management setup.

See also T242835#6064656, which observes FeatureManager as one such service in violation of this principle, and T288113#7345667 which flags it as a possible suspect.

Event Timeline

Hi @Krinkle I added some additionally logging to the script, and it shows that mw.config.get('skin') is mismatched with mw.user.options.get('skin') for all events where this happens. This is a bit confusing to me, as we're using the user preference as a gauge of whether the user expects the new Vector experience (not config).

I must admit I don't know much about our preferences code (I'm digging more into that now).

From what I can see mw.config.get('skin') !== mw.user.options.get('skin') in two circumstances:

  1. URLs with a query string parameter set e.g. https://en.wikipedia.org/wiki/Spain?useskin=monobook
  2. If a hook updates the user option during the rendering of the skin e.g.
$wgHooks['SidebarBeforeOutput'][] = function ( $skin, array &$links ) {	
	$user = $skin->getUser();
	if ( $user->isRegistered() ) {	
		$optionsManager = MediaWiki\MediaWikiServices::getInstance()->getUserOptionsManager();
		$optionsManager->setOption(
			$user,
			'skin',
			'vector'
		);
		$optionsManager->saveOptions( $user );
	}
};
  1. mw.user.options.set( 'skin' was called in JavaScript

Are there other circumstances this could occur that I am missing?

Config comes from ResourceLoader::getSiteConfigSettings inside the mediawiki.base module

user config is originally seeded from user defaults inside the same module ResourceLoader::getUserDefaults which uses DefaultOptionsLookup which can be modified by the UserGetDefaultOptions hook.

The module user.options (ResourceLoaderUserOptionsModule) is injected in the HTML of the page. This is executed inside window.RLQ after the mediawiki.base module has finished loading. It uses mw.user.options.set to override those defaults. Since default skin for French Wikipedia is Vector 2022, what we must be seeing here is the result of the second call.

I added a new event unexpected_vector22__localexception_ in an attempt to work out if Special:GlobalPreferences or local overrides are playing a part here. This revealed they weren't.

I looked at the referrer URLs and saw 417 distinct users impacted in the last 24hrs. For one user, there were 252 events so obviously this is a code path that can be repeated in the same session. With help from Clare, I looked at two of those users, and for both users we discovered they have the skin preference set as empty string in the database. 441002 rows in the preferences table were set to empty string [1].

In my local version of MediaWiki I manually inserted a row for my username where skin was set to empty string. Interestingly, after doing that mw.user.options.get('skin') returned empty string. I was expecting this to be 'vector' thanks to some kind of normalization.

I looked again at @Krinkle's script and wondered what happened if the user preference for skin was empty string. Turns out it logs in that situation.

I added a log event to the script for empty string (to see how many of the logs could be explained by an empty string preference). Sure enough all events seem to be coming from users in this state (to be confirmed with more data over time).

So from this I can conclude that the original premise is incorrect. The script is not telling us that "Users who have selected legacy Vector, seeing Vector 2022 on certain page views". Instead, it is telling us that we are storing garbage in the preferences table. Let's continue that investigation on T306056.

[1] select count(*) from user_properties where up_property = 'skin' and up_value = '';

The script is not telling us that "Users who have selected legacy Vector, seeing Vector 2022 on certain page views". Instead, it is telling us that we are storing garbage in the preferences table. […]

Indeed. It would seem that after the bulk of the failed expectations were, after many months, fixed by splitting of the skins and the subsequent removal of the migration mode. What remained were false positives from Skin::normalizeKey being applied to the effectively chosen skin, but not the (stored) preference value provided by the user options service. This is expected, but indeed led to my rudimentary instrumentation falling short.

There's some history here that may be worth reading up on, as this is actually not the time time we've encountered this: