Fix config override of other settings levels

src/settings/Settings.tsx

@@ -134,15 +134,13 @@ export interface IBaseSetting<T extends SettingValueType = SettingValueType> {
     isFeature?: false | undefined;
 
     /**
-     * If true, then the presence of this setting in `config.json` will disable the option in the UI.
+     * If true, then the presence of this setting in `config.json` will cause the config level to take precedence over settings at other levels and will disable the option in the UI.
      *
      * In other words, we prevent the user overriding the setting if an explicit value is given in `config.json`.
-     * XXX:  note that users who have already set a non-default value before `config.json` is update will continue
-     * to use that value (and, indeed, won't be able to change it!): https://github.com/element-hq/element-web/issues/26877
      *
      * Obviously, this only really makes sense if `supportedLevels` includes {@link SettingLevel.CONFIG}.
      */
-    configDisablesSetting?: true;
+    configOverridesSetting?: true;
 
     // Display names are strongly recommended for clarity.
     // Display name can also be an object for different levels.
@@ -268,7 +266,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_msc3531_hide_messages_pending_moderation": {
         isFeature: true,
         labsGroup: LabGroup.Moderation,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         // Requires a reload since this setting is cached in EventUtils
         controller: new ReloadOnChangeController(),
         displayName: _td("labs|msc3531_hide_messages_pending_moderation"),
@@ -278,7 +276,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_report_to_moderators": {
         isFeature: true,
         labsGroup: LabGroup.Moderation,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|report_to_moderators"),
         description: _td("labs|report_to_moderators_description"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
@@ -287,23 +285,23 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_latex_maths": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|latex_maths"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_pinning": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|pinning"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_wysiwyg_composer": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|wysiwyg_composer"),
         description: _td("labs|feature_wysiwyg_composer_description"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
@@ -312,7 +310,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_mjolnir": {
         isFeature: true,
         labsGroup: LabGroup.Moderation,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|mjolnir"),
         description: _td("labs|currently_experimental"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
@@ -321,15 +319,15 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_custom_themes": {
         isFeature: true,
         labsGroup: LabGroup.Themes,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|custom_themes"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
     },
     "feature_dehydration": {
         isFeature: true,
         labsGroup: LabGroup.Encryption,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|dehydration"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
@@ -350,23 +348,23 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_html_topic": {
         isFeature: true,
         labsGroup: LabGroup.Rooms,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|html_topic"),
         default: false,
     },
     "feature_bridge_state": {
         isFeature: true,
         labsGroup: LabGroup.Rooms,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|bridge_state"),
         default: false,
     },
     "feature_jump_to_date": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|jump_to_date"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         default: false,
@@ -398,7 +396,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_sliding_sync": {
         isFeature: true,
         labsGroup: LabGroup.Developer,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|sliding_sync"),
         description: _td("labs|sliding_sync_description"),
@@ -414,7 +412,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_element_call_video_rooms": {
         isFeature: true,
         labsGroup: LabGroup.VoiceAndVideo,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|element_call_video_rooms"),
         controller: new ReloadOnChangeController(),
@@ -423,7 +421,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_group_calls": {
         isFeature: true,
         labsGroup: LabGroup.VoiceAndVideo,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|group_calls"),
         controller: new ReloadOnChangeController(),
@@ -432,15 +430,15 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_disable_call_per_sender_encryption": {
         isFeature: true,
         labsGroup: LabGroup.VoiceAndVideo,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|feature_disable_call_per_sender_encryption"),
         default: false,
     },
     "feature_allow_screen_share_only_mode": {
         isFeature: true,
         labsGroup: LabGroup.VoiceAndVideo,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         description: _td("labs|under_active_development"),
         displayName: _td("labs|allow_screen_share_only_mode"),
@@ -450,7 +448,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_location_share_live": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|location_share_live"),
         description: _td("labs|location_share_live_description"),
@@ -460,7 +458,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_dynamic_room_predecessors": {
         isFeature: true,
         labsGroup: LabGroup.Rooms,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|dynamic_room_predecessors"),
         description: _td("labs|dynamic_room_predecessors_description"),
@@ -470,7 +468,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     [Features.VoiceBroadcast]: {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|voice_broadcast"),
         default: false,
@@ -483,7 +481,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     [Features.OidcNativeFlow]: {
         isFeature: true,
         labsGroup: LabGroup.Developer,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|oidc_native_flow"),
         description: _td("labs|oidc_native_flow_description"),
@@ -493,7 +491,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
         // use the rust matrix-sdk-crypto-wasm for crypto.
         isFeature: true,
         labsGroup: LabGroup.Developer,
-        // unlike most features, `configDisablesSetting` is false here.
+        // unlike most features, `configOverridesSetting` is false here.
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
         displayName: _td("labs|rust_crypto"),
         description: () => {
@@ -527,7 +525,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_render_reaction_images": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|render_reaction_images"),
         description: _td("labs|render_reaction_images_description"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
@@ -609,15 +607,15 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_ask_to_join": {
         isFeature: true,
         labsGroup: LabGroup.Rooms,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         default: false,
         displayName: _td("labs|ask_to_join"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
     },
     "feature_new_room_decoration_ui": {
         isFeature: true,
         labsGroup: LabGroup.Rooms,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|new_room_decoration_ui"),
         description: _td("labs|under_active_development"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
@@ -627,7 +625,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
     "feature_notifications": {
         isFeature: true,
         labsGroup: LabGroup.Messaging,
-        configDisablesSetting: true,
+        configOverridesSetting: true,
         displayName: _td("labs|notifications"),
         description: _td("labs|unrealiable_e2e"),
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,

src/settings/SettingsStore.ts

@@ -356,6 +356,13 @@ export default class SettingsStore {
      * Gets a setting's value at a particular level, ignoring all levels that are more specific.
      * @param {SettingLevel|"config"|"default"} level The
      * level to look at.
+     *
+     *  This takes into account the value of {@link IBaseSetting#configOverridesSetting} of the `SettingController`.
+     *  If {@link IBaseSetting#configOverridesSetting} is true, and the config level contains a valid value,
+     *  this value will be read from config level rather than level passed.
+     *  However the `explicit` paramter is respected, in that if true the value will only be read from the level specified.
+     *  In this case it is therefore the responsibilty of the calling code to ensure the config level setting is respected.
+     *
      * @param {string} settingName The name of the setting to read.
      * @param {String} roomId The room ID to read the setting value in, may be null.
      * @param {boolean} explicit If true, this method will not consider other levels, just the one
@@ -370,6 +377,13 @@ export default class SettingsStore {
         explicit = false,
         excludeDefault = false,
     ): any {
+        // For some config settings (mostly: non-beta features), a value in config.json overrides the local setting
+        // (ie: we force them as enabled or disabled). In this case we should read the value from the config.
+        const finalLevel: SettingLevel =
+            !explicit && level !== SettingLevel.CONFIG && this.doesConfigOverrideSetting(settingName, roomId)
+                ? SettingLevel.CONFIG
+                : level;
+
         // Verify that the setting is actually a setting
         const setting = SETTINGS[settingName];
         if (!setting) {
@@ -379,8 +393,8 @@ export default class SettingsStore {
         const levelOrder = getLevelOrder(setting);
         if (!levelOrder.includes(SettingLevel.DEFAULT)) levelOrder.push(SettingLevel.DEFAULT); // always include default
 
-        const minIndex = levelOrder.indexOf(level);
-        if (minIndex === -1) throw new Error(`Level "${level}" for setting "${settingName}" is not prioritized`);
+        const minIndex = levelOrder.indexOf(finalLevel);
+        if (minIndex === -1) throw new Error(`Level "${finalLevel}" for setting "${settingName}" is not prioritized`);
 
         const handlers = SettingsStore.getHandlers(settingName);
 
@@ -392,12 +406,12 @@ export default class SettingsStore {
         }
 
         if (explicit) {
-            const handler = handlers[level];
+            const handler = handlers[finalLevel];
             if (!handler) {
-                return SettingsStore.getFinalValue(setting, level, roomId, null, null);
+                return SettingsStore.getFinalValue(setting, finalLevel, roomId, null, null);
             }
             const value = handler.getValue(settingName, roomId);
-            return SettingsStore.getFinalValue(setting, level, roomId, value, level);
+            return SettingsStore.getFinalValue(setting, finalLevel, roomId, value, finalLevel);
         }
 
         for (let i = minIndex; i < levelOrder.length; i++) {
@@ -407,10 +421,10 @@ export default class SettingsStore {
 
             const value = handler.getValue(settingName, roomId);
             if (value === null || value === undefined) continue;
-            return SettingsStore.getFinalValue(setting, level, roomId, value, levelOrder[i]);
+            return SettingsStore.getFinalValue(setting, finalLevel, roomId, value, levelOrder[i]);
         }
 
-        return SettingsStore.getFinalValue(setting, level, roomId, null, null);
+        return SettingsStore.getFinalValue(setting, finalLevel, roomId, null, null);
     }
 
     /**
@@ -505,7 +519,7 @@ export default class SettingsStore {
      * set for a particular room, otherwise it should be supplied.
      *
      * This takes into account both the value of {@link SettingController#settingDisabled} of the
-     * `SettingController`, if any; and, for settings where {@link IBaseSetting#configDisablesSetting} is true,
+     * `SettingController`, if any; and, for settings where {@link IBaseSetting#configOverridesSetting} is true,
      * whether the setting has been given a value in `config.json`.
      *
      * Typically, if the user cannot set the setting, it should be hidden, to declutter the UI;
@@ -529,8 +543,11 @@ export default class SettingsStore {
         }
 
         // For some config settings (mostly: non-beta features), a value in config.json overrides the local setting
-        // (ie: we force them as enabled or disabled).
-        if (SETTINGS[settingName]?.configDisablesSetting) {
+        // (ie: we force them as enabled or disabled). In this case we such not let the user change the setting.
+        if (this.doesConfigOverrideSetting(settingName, roomId)) {
+            return false;
+        }
+        if (SETTINGS[settingName]?.configOverridesSetting) {
             const configVal = SettingsStore.getValueAt(SettingLevel.CONFIG, settingName, roomId, true, true);
             if (configVal === true || configVal === false) return false;
         }
@@ -540,6 +557,20 @@ export default class SettingsStore {
         return handler.canSetValue(settingName, roomId);
     }
 
+    /**
+     * Determines if the given setting's config should override other levels
+     * @param settingName
+     * @param roomId
+     * @returns
+     */
+    private static doesConfigOverrideSetting(settingName: string, roomId: string | null): boolean {
+        if (SETTINGS[settingName]?.configOverridesSetting) {
+            const configVal = SettingsStore.getValueAt(SettingLevel.CONFIG, settingName, roomId, true, true);
+            if (configVal === true || configVal === false) return true;
+        }
+        return false;
+    }
+
     /**
      * Determines if the given level is supported on this device.
      * @param {SettingLevel} level The level