Page MenuHomePhabricator

Globally set preferences no longer greyed out in local preferences
Open, Needs TriagePublic3 Estimated Story PointsBUG REPORT

Description

Steps to reproduce

  1. Set some preferences in your global preferences if you don’t have any yet.
  2. Go to your local preferences and look at the preference you made global.

Actual results

  1. There’s an unchecked Set a local exception for this global preference. checkbox below the globally set preference, but the local preference isn’t greyed out.
    Screenshot_2021-07-07 Preferences - Wikipédia.png (268×428 px, 42 KB)

Expected results

  1. There’s an unchecked Set a local exception for this global preference. checkbox below the globally set preference, and the local preference is greyed out.

Summary

Root cause: GlobalPreferences relied on a feature prior to https://gerrit.wikimedia.org/r/c/700722, which is preferences wouldn't be saved when lacking default value and the value is false.
A bunch of false local exceptions will be saved when users modify their local preferences, maybe we should figure out how to set default values later.

Software version

I think it was okay before MW 1.37.0-wmf.12.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Not just a huwiki problem, occurs on all wikis.

Change 682214 had a related patch set uploaded (by Func; author: Func):

[mediawiki/core@master] Use native method to disable new users checkbox in Special:Preferences

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

Change 683142 had a related patch set uploaded (by Func; author: Func):

[mediawiki/extensions/GlobalPreferences@master] Use native method to disable new users checkbox

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

Change 682303 had a related patch set uploaded (by Func; author: Func):

[mediawiki/core@master] Add support for conditional disable fields in HTMLForm

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

From the merged task - this doesn't appear to be a UI display bug, the GPrefs do indeed to not actually be working from a user story perspective - but that could be a different problem? (Perhaps from T294675)

I think it's a UI bug in the sense the boxes aren't grayed out, and hence are passed in the POST request even if unchanged. This seems to be the same problem described at T289931. It's probably the same as T294675, too, but I'm not sure. Either way we've got some patches now up for review now that hopefully will fix it. Sorry for the delay!

Change 737539 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/GlobalPreferences@master] Set initial state of local-exception disabling of preference

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

I'm not sure if it's worth it, but if we do want to add a short-term workaround for this, I think this would suffice: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GlobalPreferences/+/737539 The fix by @Func above of adding support in core for disable-if is better, of course (but more complicated).

I can reproduce this in production, but not locally. If I set a global preference and go to Special:Preferences to set a local exception, the preference field is greyed out as expected. @Samwilson any idea about that?

No, that's strange. With current master of core and GP, and no weird caching things going on? I can replicate it locally with Firefox and Chrome, every time.

No, that's strange. With current master of core and GP, and no weird caching things going on? I can replicate it locally with Firefox and Chrome, every time.

Yeah, I'm using master of GP and a slightly outdated version of MW core (a couple days old), no caching but cannot reproduce. This line disables the field for me.

I changed the content and interface languages to italian, and now it works. The "gender" field is no longer disabled. WTF?!

And now I set English again and I can reproduce the bug... Nevermind. I've verified that the quick fix above is enough for the immediate problem, so I'm going to merge it. The core change with disable-if is still something that would be nice to have, but for now I want to fix the issue ASAP to try and understand how all these bug reports about GP are related.

Yeah, I'm using master of GP and a slightly outdated version of MW core (a couple days old), no caching but cannot reproduce. This line disables the field for me.

Wait, with this line, the fields should be greyed out even disable js in the browser. It works fine on mediawiki.org and the huwiki mentioned above for me, but I can reproduce the bug on enwiki and zhwiki...

There should be some deeper problems.

Ok, I see. I set a local exception and save it, then all the fields were enabled initially after refresh.

So there might be many useless fields written to the database.

@NRodriguez The merged-in tasks about "emails from new users" are different from this one, and they were created way before this.

Can they be restored and can I move my patches for them back to one of them?

Change 737820 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/GlobalPreferences@master] UserOptionsLookup for local exception boolean values

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

Change 737539 merged by jenkins-bot:

[mediawiki/extensions/GlobalPreferences@master] Set initial state of local-exception disabling of preference

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

Change 737820 merged by jenkins-bot:

[mediawiki/extensions/GlobalPreferences@master] Handle loosly-boolean local exception values

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

Change 737539 merged by jenkins-bot:

[mediawiki/extensions/GlobalPreferences@master] Set initial state of local-exception disabling of preference

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

Change 737820 merged by jenkins-bot:

[mediawiki/extensions/GlobalPreferences@master] Handle loosly-boolean local exception values

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

@NRodriguez These changes merged last week will go live starting tomorrow. So I did a bit of testing. This bug has left a window where users might get their local and global preferences into an inconsistent state. I wanted to know whether this change would make it easy for a user to fix any inconsistencies.

Before this change:

  1. If an option is global, after I save the Special:Preferences form the option is no longer disabled in Special:Preferences (as shown below and described in the bug description)
    inconsistency_1_step_1.png (319×801 px, 35 KB)
  2. I can save a new value for this option in Special:Preferences and this gets saved as a "local" setting in user_properties table, without having to check "set a local exception...". However, because "set a local exception..." has not been set it will not actually be applied on that wiki. It will just look like that when you look at Special:Preferences.
    inconsistency_1_step_2.png (314×789 px, 35 KB)
  3. In Special:GlobalPreferences, it will show the "local" value for this option that you set in step 2, even though this is not the global value (which is saved in the global_preferences table)
    inconsistency_1_step_3.png (277×877 px, 31 KB)
  4. This might lead to users thinking the global value is different from what it actually is, or accidentally saving a different value globally to the one they wanted originally

After this change:

  1. Special:GlobalPreferences correctly reflects the global value for the option which is in the global_preferences table
    inconsistency_1_step_5.png (268×871 px, 30 KB)
  2. In Special:Preferences the option is disabled and shows the global value of the option. This correctly reflects the fact that this is the value that will be applied on this wiki.
    inconsistency_1_step_6.png (308×783 px, 34 KB)
  3. If they then remove the global value for that option in Special:GlobalPreferences:
    • In Special:GlobalPreferences the option is disabled and shows the "local" value
      inconsistency_1_step_7a.png (266×853 px, 30 KB)
    • Special:Preferences will show the "local" value from user_properties table which was set in step 2
      inconsistency_1_step_7b.png (275×745 px, 30 KB)

If in step 2 you have checked "set local exception...", after this change Special:Preferences will correctly show the local value of the option and Special:GlobalPreferences will correctly show the global value of the option.

If they have accidentally saved the wrong value globally in step 4, I don't think there is much we can do about this. I don't see how we can distinguish between an option set in error and one set deliberately.

I tested this for a few different types of UI element which you find on the Special:Preferences form (including dropdown, checkbox, radio, counter) but not for all of them.

It should be noted that there are a lot of UI bugs in Special:Preferences and Special:GlobalPreferences and this change has not fixed all of them. Many are long-standing and can make the behaviour of the form seem unpredictable. For example, the options as they appear in the form may not correctly reflect the real values in the database. Remember to do a hard refresh or use the forms without javascript to make sure that you are seeing the correct values.

Test environment local MediaWiki install where I was switching between:

  • GlobalPreferences 0.1.2 (e40f5fb) 15:18, 9 November 2021 (the version before these changes)
  • GlobalPreferences 0.1.2 (f317f65) 16:05, 11 November 2021 (the version after these changes)

Test browser Firefox 78

This comment was removed by Func.

@Samwilson mind back-estimating this work? Thank you!

While the recent fix seems to have made global preferences grayed out in local preferences, as expected, I'm finding there's a bug with the opposite – where when I attempt to make a local exception for some global preferences, the checkbox remains grayed out when it shouldn't. I've filed T296028: Unable to set a local override for some Global Preferences for that. I'm not sure if it's distinct to the bug described in this task, or a regression. I've tried disabling/enabling the global preference to ensure the settings aren't in a consistent state like Dom was talking about in T286271#7503827.

Samwilson set the point value for this task to 3.Nov 24 2021, 1:27 AM
Func added a subscriber: Func.

Change 683142 had a related patch set uploaded (by Func; author: Func):

[mediawiki/extensions/GlobalPreferences@master] Use native method to disable checkboxes conditionally

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

Oh, sorry, it's a mistake when I rebase.

All related patches are merged and the main issue described here seems to be fixed. I'm guessing this was still in the "Needs Review" column because https://gerrit.wikimedia.org/r/682214 was attached to this task, but that's since been moved to T272302. I know Dom already did a pass with T286271#7503827 but I'll move to QA just in case. As long as we're confident the main issue as described here is fixed, I think we're okay to move this to Product Sign-Off. There are of course still a number of outstanding issues with GlobalPrefs but those can be addressed in separate tasks.

I haven't seen any user option which are not correctly disabled when they are set globally.

Test environment: https://en.wikipedia.beta.wmflabs.org, https://wikidata.beta.wmflabs.org MediaWiki 1.38.0-alpha (2682461) 07:22, 2 December 2021. GlobalPreferences 0.1.2 (e9ea9af) 05:41, 1 December 2021.

Thanks for links @dom_walden

Test environment: https://en.wikipedia.beta.wmflabs.org, https://wikidata.beta.wmflabs.org MediaWiki 1.38.0-alpha (2682461) 07:22, 2 December 2021. GlobalPreferences 0.1.2 (e9ea9af) 05:41, 1 December 2021.

In my user preferences in Chinese Wikipedia I see all items from the "beta" section suffering from this problem.

Note: Chinese Wikipedia recently had VisualEditor moved out of beta.

Screenshot 2021-12-24 181158.png (924×1 px, 110 KB)

In my user preferences in Chinese Wikipedia I see all items from the "beta" section suffering from this problem.

This is a different problem, if you can file another task, that would be helpful. (Title of this task is too extensive...)

As I can see, that's because NewHTMLCheckField is not infusible, and contains no 'disable-if' information.

We've also seen similar at Miraheze (https://phabricator.miraheze.org/T8554) but only affecting 1 known user (@dmehus). Is there a reason this would only affect certain browsers or could it be another bug.

Vivaldi	5.0.2497.32 (Stable channel) (64-bit)
Revision	4891cb2a30270ebb895cf22eb99642090bde0046
OS	Windows 10 Version 21H1 (Build 19043.1415)
JavaScript	V8 9.6.180.21
User Agent	Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.113 Safari/537.36
Command Line	"REDACTED" --flag-switches-begin --flag-switches-end --origin-trial-disabled-features=CaptureHandle --save-page-as-mhtml
Executable Path	C:\Users\REDACTED\AppData\Local\Vivaldi\Application\vivaldi.exe
Profile Path	C:\Users\REDACTED\AppData\Local\Vivaldi\User Data\Default
Product	Version
MediaWiki	1.37.1 (90b4146)
14:54, 30 December 2021
PHP	7.3.31-1~deb10u1 (fpm-fcgi)
MariaDB	10.4.21-MariaDB-1:10.4.21+maria~buster-log
ICU	63.1
LuaSandbox	3.0.3
Lua	5.1.5
Pygments	2.10.0Product	Version
MediaWiki	1.37.1 (90b4146)
14:54, 30 December 2021
PHP	7.3.31-1~deb10u1 (fpm-fcgi)
MariaDB	10.4.21-MariaDB-1:10.4.21+maria~buster-log
ICU	63.1
LuaSandbox	3.0.3
Lua	5.1.5
Pygments	2.10.0
GlobalPreferences	0.1.2 (8c3eb0c) 05:49, 8 December 2021

We've also seen similar at Miraheze (https://phabricator.miraheze.org/T8554) but only affecting 1 known user (@dmehus). Is there a reason this would only affect certain browsers or could it be another bug.

Hi, you should try to modify your local preferences and save once. I believe you can reproduce after that.

Thanks, I'm now able to reproduce it. Would you be able to backport the fixes @Func to 1.37 ?

@RhinosF1 You mean my patch for T294675? There are some follow-up bugs (T298003) and may need some core change.
I'm not familiar with the backport policy, and that patch should be rebased on patches for this task. Maybe someone can backport both of them.

Change 751189 had a related patch set uploaded (by MusikAnimal; author: Samwilson):

[mediawiki/extensions/GlobalPreferences@REL1_37] Handle loosly-boolean local exception values

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

Change 751190 had a related patch set uploaded (by MusikAnimal; author: Samwilson):

[mediawiki/extensions/GlobalPreferences@REL1_37] Set initial state of local-exception disabling of preference

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

Change 751190 merged by jenkins-bot:

[mediawiki/extensions/GlobalPreferences@REL1_37] Set initial state of local-exception disabling of preference

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

Change 751189 merged by jenkins-bot:

[mediawiki/extensions/GlobalPreferences@REL1_37] Handle loosly-boolean local exception values

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