From what I understand about global preferences - they should just work. It's possible there is a problem with if preferences related to T246491.

I've set the global preference as indicated by the UI of the form but it has not altered the design so there is something wrong with how that value is read but without understanding the innerworkings of GlobalPreferences I can't shed light on why:

Nobody in this team is familiar with this extension so we will likely need to find one of the maintainers to analyze and understand this problem.

Pinging folks on the Community Tech team who may have insights around Global Preferences.

@MusikAnimal & @Samwilson: Any idea why "Use Legacy Vector" isn't working as a Global Preference?

Also adding @dmaza, so he's aware of the conversation.

I think T246491: Special:GlobalPreferences does not deal well with "conditional preferences" (using hide-if) is the root cause for this.

Last week, Tech News had something about enabling Legacy Vector using globalPrefs. Let's have a note about it not working (and hopefully now fixed) in the next issue!

I wonder if this bug I see on logstash is related?

https://gerrit.wikimedia.org/g/mediawiki/extensions/GlobalPreferences/+/457e5697a0bbbb4106c0e7a8d507b7b3c81a96e9/resources/ext.GlobalPreferences.global.js#39

TypeError: sectionCheckbox is undefined
t stack_trace 	
	at updateSelectAllCheckboxState

This is blocked on T246491

cc @Samwilson The fix for T246491 doesn't seem to have helped here unfortunately.

Looking at the POST request I'm seeing:

wpVectorSkinVersion-global	"1"

however I'm not seeing the checkbox value in the POST request (I only see it when the checkbox is checked):

wpVectorSkinVersion	"0"

so the skin version is never set when the checkbox is not checked. Is it possible that GlobalPreferences dropping '0' values?

Change 618385 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Fix global preference for Vector skin version

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

Looking at this more closely, I'm seeing an empty checkbox equate to '0' rather than using the 'default' value in the checkbox definition. Is it possible as well as hide-if
I've articulated what I think the problem here is - https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GlobalPreferences/+/618171/2#message-95434914c49544749b00ee8ab2e0d98fd891455c - @Samwilson would you be able to confirm?

In T258493#6361028, @Jdlrobson wrote:

however I'm not seeing the checkbox value in the POST request (I only see it when the checkbox is checked)

This behaviour is expected, e.g. see the note in this section of the documentation on MDN.

In T258493#6361143, @Jdlrobson wrote:

Looking at this more closely, I'm seeing an empty checkbox equate to '0' rather than using the 'default' value in the checkbox definition.

Could you elaborate on this? Where are you seeing this?

http://wikidata-reading-web-staging.wmflabs.org/wiki/Special:GlobalPreferences
http://en-reading-web-staging.wmflabs.org/wiki/Special:GlobalPreferences

In rSVECde76ab59c143: [Special:Preferences] [PHP] Add skin version user preference and configs, we added the skin version user preference to the Special:Preferences page (and therefore to Special:GlobalPreferences as well). As part of that change, we added an PreferencesFormPreSave hook that adapts the value that's submitted by the browser to a value that we understand – Constants::SKIN_VERSION_LATEST or Constants::SKIN_VERSION_LEGACY – and sets the user preference. However, this hook is not run when the Special:GlobalPreferences page is submitted and so the global preference is persisted and read unadapted.

Edit: The Special:GlobalPreferences page doesn't end up invoking UserOptionsManager::saveOptions.

AFAICT we're going to need to add a hook equivalent to UserSaveOptions to GlobalPreferences\GlobalPreferencesFactory:

namespace GlobalPreferences\Hook;

/**
 * @stable for implementation
 * @ingroup Hooks
 */
interface UserSaveGlobalOptionsHook {

  /**
    * @param User $user The identity of the logged-in user
    * @param array &$options The user's options that are going to be saved as an associative array
    * @param array $originalOptions The user's original options as an associative array
    * @return bool|void `true` or no return value to continue or `false` to abort
    */
  function onUserSaveGlobalOptions(
    User $user,
    array &$options,
    array $originalOptions
  );
}

namespace GlobalPreferences\Hook;

class HookRunner
  implements UserSaveGlobalOptionsHook
{
  public function onUserSaveGlobalOptions(
    User $user,
    array &$options,
    array $originalOptions
  ) {
    $this->container->run( 'UserSaveGlobalOptions', [ $user, $options, $originalOptions ] );
  }
}

/* ... */

public function setGlobalPreferences(
  User $user,
  $newGlobalPrefs,
  IContextSource $context,
  array $checkMatricesToClear = []
) {
  /* ... */
  
  if ( !$this->hookRunner->onUserSaveGlobalOptions( $user, &$newGlobalPrefs, $knownPrefs ) ) {
    return false;
  }

  $this->save( $newGlobalPrefs, $knownPrefs, $checkMatricesToClear );

  $user->clearInstanceCache();

  return true;
}

Then, in Vector, we can add a handler for this hook and adapt the value for the VectorSkinVersion global preference in the same way that we adapt the value for the local preference:

/* ... */
public static function onUserSaveGlobalOptions( User $user, &$options, $originalOptions ) {
  self::onUserSaveOptions( $user, $options, $originalOptions );
}

public static function onUserSaveOptions( User $user, &$options, $originalOptions } {
$isVectorEnabled = ( $options[ 'skin' ] ?? '' ) === Constants::SKIN_NAME;
  if ( $isVectorEnabled && array_key_exists( Constants::PREF_KEY_SKIN_VERSION, $options ) ) {
    // A preference was set. However, Special:GlobalPreferences converts the result to a boolean when a
    // version name string is wanted instead. Convert the boolean to a version string in case the
    // preference display is changed to a list later (e.g., a "_new_ new Vector" / '3' or
    // 'alpha').
    $options[ Constants::PREF_KEY_SKIN_VERSION ] = $options[ Constants::PREF_KEY_SKIN_VERSION ] ?
      Constants::SKIN_VERSION_LEGACY :
      Constants::SKIN_VERSION_LATEST;
    } elseif ( array_key_exists( Constants::PREF_KEY_SKIN_VERSION, $originalOptions ) ) {
      // The setting was cleared. However, this is likely because a different skin was chosen and
      // the skin version preference was hidden.
      $options[ Constants::PREF_KEY_SKIN_VERSION ] = $originalOptions[ Constants::PREF_KEY_SKIN_VERSION ];
    }
}

Change 618587 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Test: SkinVersionLookup fails for opted out users

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

@phuedx Some alternative ideas, maybe better, maybe worse:

• Could we remove the PreferencesFormPreSave hook, and just use the normal values for the preference? You'd have to handle the existing values saved in the database when checking which version of the skin is enabled, but that shouldn't be difficult. Having a mix of different values might make it harder to analyze the aggregated data with SQL queries though, if you need to do that.
• Could we make a subclass of HTMLCheckField and use a field of that custom type when defining the preference? Then you could have the code to "remap" the values in that subclass in loadDataFromRequest(), rather than the hook. I'm not sure if this would be easier or harder than what you proposed, but it might be worth investigating.

In T258493#6366573, @matmarex wrote:

@phuedx Some alternative ideas, maybe better, maybe worse:

• Could we remove the PreferencesFormPreSave hook, and just use the normal values for the preference? You'd have to handle the existing values saved in the database when checking which version of the skin is enabled, but that shouldn't be difficult. Having a mix of different values might make it harder to analyze the aggregated data with SQL queries though, if you need to do that.

I'd initially discarded this solution because of the issue you've identified – having a mix of different values to handle during analysis – but this is actually the status quo due to this bug. However, I think it's still reasonable to discard this solution in favour of the one below as it requires the analyst (or MediaWiki/Wikimedia data archeologist of the future) to also decode the value.

• Could we make a subclass of HTMLCheckField and use a field of that custom type when defining the preference? Then you could have the code to "remap" the values in that subclass in loadDataFromRequest(), rather than the hook. I'm not sure if this would be easier or harder than what you proposed, but it might be worth investigating.

Neato! I think this is well worth investigating. On reflection I think I reached for the hook-based solution as it's striking that (rightly or wrongly) GlobalPreferences is missing hooks equivalent to UserSaveOptions, UserSaveSettings, and PreferencesFormPreSave.

Change 619475 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/skins/Vector@master] [WIP] [Special:Preferences] Add HTMLSkinVersionField field

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

Change 618385 abandoned by Jdlrobson:
[mediawiki/skins/Vector@master] Fix preference handling for Vector skin version

Reason:

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

Change 618587 abandoned by Jdlrobson:
[mediawiki/skins/Vector@master] Test: SkinVersionLookup fails for opted out users

Reason:
This has been folded into Sam's patch https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/ /619475

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

Change 621258 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/extensions/GlobalPreferences@master] GlobalPreferencesFactory: Allow Vector's HTMLLegacySkinVersionField class

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

Change 619475 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [Special:Preferences] [PHP] Add HTMLSkinVersionField form field

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

Change 621258 merged by jenkins-bot:
[mediawiki/extensions/GlobalPreferences@master] GlobalPreferencesFactory: Allow Vector's HTMLLegacySkinVersionField class

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

Test Result - Prod

Status: ✅ PASS
Environment: enwiki
OS: macOS Catalina
Browser: Chrome
Device: MBP
Emulated Device: NA
Test Artifact(s):

QA steps

1. Go to Special:GlobalPreferences
2. Make Use Legacy Vector a global setting (check the checkbox to the left)
3. Opt-out in order to use our new Vector globally (uncheck the checkbox to the right)
4. Go to any non-preference page on any wiki - the setting doesn't work.

Hello, @ovasileva, when it should start to work? It does not now.

In T258493#6458846, @IKhitron wrote:

Hello, @ovasileva, when it should start to work? It does not now.

@IKhitron - it's working for me right now. What steps are you taking to turn it on from the global preferences page?

@IKhitron I had to resave the global preference to get this to work and disable an override on some of the Wikipedias. Maybe you need to do the same?

Jdlrobson, I'm still not there.

In T258493#6458870, @ovasileva wrote:

@IKhitron - it's working for me right now. What steps are you taking to turn it on from the global preferences page?

Scenario A.

1. Open Special:GlobalPreferences -> Appearance.
2. I can see now all the radioboxes marked.
3. But "Select options below to be global" box is "-", not "V". I can see the same problem on "Notifications".

Scenario B.

1. Open Special:GlobalPreferences -> Appearance.
2. Use Legacy Vector left "use global" box is on, right "use legacy vector" box is off.
3. Turn the right box on.
4. Save.

Expected: the right box is on.
Got: the right box is still off.

See also T262879.