Page MenuHomePhabricator

Add hook that allows extensions to prevent preferences from getting exported to client-side
Closed, ResolvedPublic

Description

In includes/resourceloader/ResourceLoaderUserOptionsModule.php we have:

$userOptionsLookup = MediaWikiServices::getInstance()->getUserOptionsLookup();
$options = $userOptionsLookup->getOptions( $user, UserOptionsLookup::EXCLUDE_DEFAULTS );
// Optimisation: Only output this function call if the user has non-default settings.
if ( $options ) {
	$script .= 'mw.user.options.set(' . $context->encodeJson( $options ) . ');';
}

It could be nice if we had a flag like GETOPTIONS_EXCLUDE_SERVERSIDE_PREFS, then in UserOptionsManager we could run a hook that asks extensions if any of their defined options should not be exported to the client-side, and maybe a Context object could be passed so that extensions could decide if the option should be exported client-side in some circumstances but not others. This could help with performance issues that arise when extensions use preferences for managing data that doesn't need to be present on each page.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Being included in mw.user.options is the biggest performance issue with user options, but it's not the only one: all options for the current user are retrieved from the database on every MediaWiki request. That's ideal for configuration settings which influence how most pages are rendered for the user, but for options which are rarely accessed, something like T128602: RFC: Backend for synchronized data from Wikipedia mobile apps would be a cleaner approach, IMO.

One hacky approach that came up in past discussions was to filter out preferences with a certain prefix (much like how prefs with the userjs- prefix get special handling). I'm not a fan, but it would provide the same benefit as a hook (I don't think there's any use case where the hook would have more complicated decision logic than checking if the preference name matches some blacklist or pattern) with less setup.

For large values relating to features we don't yet have today, there was indeed an RFC to create a separate store. Entirely separate from user preferences. (At least from MW's runtime perspective). That RFC, T128602, is stalled as the original use case for it is no longer in development and/or went with another approach (I think?).

@kostajh What specific use case is the motivation for this task?

In terms of exclusion/inclusion, I think that makes sense and I'd actually like us to go a step further and require an enabled component to specifically register server-side its need to preload a particular user option for the current page. Noting that from the JS perspective, preloaded user options are an optimisation. They are not the only way to get the data. (We have an API for that.)

What specific use case is the motivation for this task?

We have a few.

  • Special:WelcomeSurvey where the user's submissions are stored as serialized JSON in a user option.
  • Echo's echo-notifications-page-linked-title-muted-list has a list of article title IDs that are used only server side and have no need for use on client-side
  • GrowthExperiments Help Panel feature stores serialized JSON with a list of the three most recent questions for the help desk and three most recent questions for the mentor module. We implemented that in the absence of a structure discussion system. That one is (IIRC) used client-side but only on Special:Homepage or when the help panel is open so it would be nice to restrict exporting that client-side for those particular cases

What specific use case is the motivation for this task?

We have a few.

One more: MediaWiki-extensions-GlobalWatchlist stores options as serialized JSON, but the options only need to be exposed on one special page, and outside of that should never be needed

Krinkle triaged this task as Medium priority.Aug 18 2020, 1:31 AM
Krinkle moved this task from Inbox to Backlog on the MediaWiki-ResourceLoader board.

There are also lots of core options that are always being exported even if they aren't needed by any (core or extension loaded) javascript.
Since it is impossible to know which options user scripts are going to want access to, perhaps mw.user.options.get could be wired up to fetch from the api if the option isn't already set?

Since it is impossible to know which options user scripts are going to want access to, perhaps mw.user.options.get could be wired up to fetch from the api if the option isn't already set?

It's a synchronous method; sync fetches are bad.

Thanks Kosta, so those are cases we'd potentially exclude to reduce page load cost.

We haven't seen this cause a big impact yet, but also haven't proactive looked for it. Either way, I'd be accept such a change. Might be able to schedule it in a few quarters, but if not might be able to collaborate or review.

Change 752369 had a related patch set uploaded (by SD0001; author: SD0001):

[mediawiki/core@master] Add hook to allow extensions to prevent preferences being exported client-side

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

Change 759005 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] resourceloader: Add unit test for ResourceLoaderUserOptionsModule

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

Change 759005 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Add unit test for ResourceLoaderUserOptionsModule

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

Change 752369 merged by jenkins-bot:

[mediawiki/core@master] Add hook to allow extensions to prevent preferences being exported client-side

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

Change 761520 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/core@master] Add test for ResourceLoaderExcludeUserOptions hook

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

Change 761520 merged by jenkins-bot:

[mediawiki/core@master] Add tests for ResourceLoaderExcludeUserOptions hook

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