Unable to change user settings
Closed, ResolvedPublic

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 5 2017, 11:28 AM
Nikerabbit triaged this task as Unbreak Now! priority.Oct 5 2017, 11:28 AM
Restricted Application added subscribers: Liuxinyu970226, Jay8g, TerraCodes. · View Herald TranscriptOct 5 2017, 11:28 AM

Git bisect says 7ab57ba290f670847f981d5fc2c79339f1d4844e – and that seems likely indeed.

This patch is only in master (which I just updated translatewiki.net today).

In Setup.php, $wgUser is initialized before $wgLang, but $wgUser represents an anonymous user at that point, there is setUser() method but that is not called. I am not sure what changes $this->user in request context between the first getUser() call and when getLanguage() is called later via StubUserLang.

Nikerabbit added a comment.EditedOct 5 2017, 12:44 PM

The culprit seems to be this code which messes with the status of the User object forcing it back to anonymous:

		if ( !$wgFullyInitialised && $this->mFrom === 'session' ) {
			var_dump( 'called too early' );
			\MediaWiki\Logger\LoggerFactory::getInstance( 'session' )
				->warning( 'User::loadFromSession called before the end of Setup.php', [
					'exception' => new Exception( 'User::loadFromSession called before the end of Setup.php' ),
				] );
			$this->loadDefaults();
			$this->mLoadedItems = $oldLoadedItems;
			return;
		}

It conveniently logs a warning... which doesn't show anywhere in my logs. Just removing this code "solves" the issue for me, but I can't tell if there are side-effects or if it breaks on some other setups. One idea is to move initialization of $wgLang (and co.) to later part. It should be okay to enforce that code that is run very early doesn't depend on these globals (except if there are hooks, then it is harder to enforce).

Elitre added a subscriber: Elitre.Oct 5 2017, 4:04 PM

Change 382478 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/core@master] Revert "Stop stubbing StubUserLang"

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

Addshore renamed this task from Unable to change interface language to Unable to change user settings.
Addshore added a subscriber: Andrew-WMDE.

I vote for reverting this now and then working on a fixed up version later.

This is currently breaking user prefs on translatewiki along with a bunch of browser tests for extensions that depend on user prefs, and probably more.

Change 382478 merged by jenkins-bot:
[mediawiki/core@master] Revert "Stop stubbing StubUserLang"

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

Change 382496 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/core@master] Revert "Remove some remaining mentions of unstubbing from core"

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

Change 382498 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/ParserFunctions@master] Revert "Language objects aren't stubbed anymore"

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

Change 382497 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Flow@master] Revert "Remove mentions of StubObject, just for typehinting"

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

Change 382499 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Flow@master] Revert "Remove references to StubUserLang, it's going away"

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

Change 382500 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/JsonConfig@master] Revert "Remove references to StubUserLang, it's going away"

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

Change 382497 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Revert "Remove mentions of StubObject, just for typehinting"

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

Change 382500 merged by jenkins-bot:
[mediawiki/extensions/JsonConfig@master] Revert "Remove references to StubUserLang, it's going away"

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

Change 382496 merged by jenkins-bot:
[mediawiki/core@master] Revert "Remove some remaining mentions of unstubbing from core"

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

Change 382498 merged by jenkins-bot:
[mediawiki/extensions/ParserFunctions@master] Revert "Language objects aren't stubbed anymore"

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

Change 382499 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Revert "Remove references to StubUserLang, it's going away"

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

Anomie added a subscriber: Anomie.Oct 5 2017, 6:02 PM

It conveniently logs a warning... which doesn't show anywhere in my logs.

It's the 'session' channel. Either you're throwing away that channel specifically, or you're throwing away all log messages to channels you haven't manually set up with a destination.

Just removing this code "solves" the issue for me, but I can't tell if there are side-effects or if it breaks on some other setups.

Over the years, we've had various bugs caused by some extension triggering the load of the user from session data early and causing other extensions' code for doing that load to blow up because things aren't initialized. As part of SessionManager, we made it so a User object loaded from the session would be an anonymous user until everything is set up.

See also T43201: UserLoadFromSession considered evil.

One idea is to move initialization of $wgLang (and co.) to later part.

The place you'd have to move it to would be below $wgFullyInitialized = true.

An alternative to just moving it would be to set it to $wgContLang at the current place and re-set it to the actual user language after $wgFullyInitialized.

It should be okay to enforce that code that is run very early doesn't depend on these globals (except if there are hooks, then it is harder to enforce).

Anything that actually uses $wgLang that early is already hitting the log warning. I see T124367: User::loadFromSession called before the end of Setup.php currently has two open subtasks.

There might also be code that's saving the StubUserLang early and unstubbing it later on.

It conveniently logs a warning... which doesn't show anywhere in my logs.

It's the 'session' channel. Either you're throwing away that channel specifically, or you're throwing away all log messages to channels you haven't manually set up with a destination.

I don't usually look at or store logs for individual requests. I watch the error log, which I expect to indicate if something is broken. I think this message should go into the error logs, given it can indicate that things are really broken (see T177524 for example).

Is this now fixed in master?

Restricted Application added a project: User-Addshore. · View Herald TranscriptOct 6 2017, 5:26 PM

(Not sure because of this bug or not. if not show me correct place) On Bengali wikipeida, yesterday i was unable to change my settings but today my whole "preferences" is automatically reset itself to the default settings. Is this supposed to happen? Should i notify other wikipedians to re-adjust their preference? they may still thinking gadget & other thing isn't working because of this bug.

Addshore moved this task from Backlog to Done ✔️ on the User-Addshore board.Oct 9 2017, 3:47 PM