Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix config override of other settings levels #12593

Merged
merged 19 commits into from
Jun 14, 2024
Merged

Fix config override of other settings levels #12593

merged 19 commits into from
Jun 14, 2024

Conversation

langleyd
Copy link
Contributor

@langleyd langleyd self-assigned this Jun 10, 2024
@langleyd langleyd marked this pull request as ready for review June 10, 2024 10:56
@langleyd langleyd requested a review from a team as a code owner June 10, 2024 10:56
@langleyd langleyd requested review from dbkr and richvdh June 10, 2024 10:56
@langleyd langleyd added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Jun 10, 2024
t3chguy
t3chguy reviewed Jun 10, 2024
View reviewed changes
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels strange to have 2 mechanisms which can make config take priority. supportedLevels being the other one as the levels are ordered IIRC

@langleyd
Copy link
Contributor Author

Yea there is a supportedLevelsAreOrdered which instructs that supportedLevels should be respected in order.

This PR was my attempt to have configDisablesSetting make a bit more sense. As i'm not sure disabling a setting for a user where there is a value in config makes sense without also enforcing that the config value is actually used(overrides the local one).

@t3chguy For all the settings with that haveconfigDisablesSetting we could instead use supportedLevelsAreOrdered and use a level order with config coming first. In this case would we expect to still need the explicit configDisablesSetting flag to disabled the setting in the UI, so it always needs to be used alongside supportedLevelsAreOrdered ? Or would you expect we add a check to see if the current setting is being overridden by config based on supportedLevelsAreOrdered and that causes the setting UI to be disabled?

@t3chguy
Copy link
Member

t3chguy commented Jun 10, 2024

Or would you expect we add a check to see if the current setting is being overridden by config based on supportedLevelsAreOrdered and that causes the setting UI to be disabled?

This feels like a nice way of doing it IMO, condensing the options we have and doing more stuff implicitly

@langleyd langleyd requested a review from t3chguy June 10, 2024 18:11
t3chguy
t3chguy reviewed Jun 10, 2024
View reviewed changes
src/settings/SettingsStore.ts Outdated Show resolved Hide resolved
src/components/views/elements/SettingsFlag.tsx Outdated Show resolved Hide resolved
src/components/views/elements/SettingsFlag.tsx Outdated Show resolved Hide resolved
@langleyd
Copy link
Contributor Author

@t3chguy I'm not sure disabling settings that have one set at a high level, necessarily makes sense in all cases. blacklistUnverifiedDevices is exposed in in room and device settings. And while room should take precedence in this case, both should be enabled.

By chance, this code didn't break things as when checking canSetValue at the device level you don't have the roomId, so doesn't see a value set at a higher level.

I wonder should we have a "higherPrecenceValuesShouldDisableSetting" property that is true by default but can be set to false?

@t3chguy
Copy link
Member

t3chguy commented Jun 12, 2024

It should probably be special-cased to config only as that's the only read-only setting level

@langleyd langleyd requested a review from t3chguy June 13, 2024 17:27
@langleyd
Copy link
Contributor Author

This seem ok now @t3chguy ?

t3chguy
t3chguy reviewed Jun 13, 2024
View reviewed changes
src/settings/SettingsStore.ts Outdated
Comment on lines 561 to 564
console.log("settingIsOveriddenAtConfigLevel");
console.log(settingName);
console.log(configIndex);
console.log(levelIndex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should probably not land

src/components/views/elements/SettingsFlag.tsx Outdated
// If a level defined in props is overridden by a level at a high presedence, it gets disabled
// and we should show the overridding value.
if (
!!SettingsStore.settingIsOveriddenAtConfigLevel(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need the !! here given its a boolean

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is identified by Sonar: https://sonarcloud.io/project/issues?id=matrix-react-sdk&pullRequest=12593&resolved=false&sinceLeakPeriod=true

@langleyd langleyd added this pull request to the merge queue Jun 14, 2024
Merged via the queue into develop with commit d6b9e2a Jun 14, 2024
30 checks passed
@langleyd langleyd deleted the langleyd/fix_config_override_setting_value branch June 14, 2024 11:22
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 8, 2024
@Midar
Update chat/element-web to 1.11.73
2567c70 
Changes in [1.11.73](https://github.com/element-hq/element-web/releases/tag/v1.11.73) (2024-08-06)
==================================================================================================
Fixes for CVE-2024-42347 / GHSA-f83w-wqhc-cfp4



Changes in [1.11.72](https://github.com/element-hq/element-web/releases/tag/v1.11.72) (2024-07-30)
==================================================================================================
## ✨ Features

* Polyfill Intl.Segmenter for wider web browser compatibility ([#27803](element-hq/element-web#27803)). Contributed by @dbkr.
* Enable audio/webaudio Modernizr rule ([#27772](element-hq/element-web#27772)). Contributed by @t3chguy.
* Add release announcement for the new room header ([#12802](matrix-org/matrix-react-sdk#12802)). Contributed by @MidhunSureshR.
* Default the room header to on ([#12803](matrix-org/matrix-react-sdk#12803)). Contributed by @MidhunSureshR.
* Update Thread Panel to match latest designs ([#12797](matrix-org/matrix-react-sdk#12797)). Contributed by @t3chguy.
* Close any open modals on logout ([#12777](matrix-org/matrix-react-sdk#12777)). Contributed by @dbkr.
* Iterate design of right panel empty state ([#12796](matrix-org/matrix-react-sdk#12796)). Contributed by @t3chguy.
* Update styling of UserInfo right panel card ([#12788](matrix-org/matrix-react-sdk#12788)). Contributed by @t3chguy.
* Accessibility: Add Landmark navigation ([#12190](matrix-org/matrix-react-sdk#12190)). Contributed by @akirk.
* Let Element Call widget receive m.room.create ([#12710](matrix-org/matrix-react-sdk#12710)). Contributed by @AndrewFerr.
* Let Element Call widget set session memberships ([#12713](matrix-org/matrix-react-sdk#12713)). Contributed by @AndrewFerr.
* Update right panel base card styling to match Compound ([#12768](matrix-org/matrix-react-sdk#12768)). Contributed by @t3chguy.
* Align `widget_build_url_ignore_dm` with call behaviour switch between 1:1 and Widget ([#12760](matrix-org/matrix-react-sdk#12760)). Contributed by @t3chguy.
* Move integrations switch ([#12733](matrix-org/matrix-react-sdk#12733)). Contributed by @dbkr.
* Element-R: Report events with withheld keys separately to Posthog. ([#12755](matrix-org/matrix-react-sdk#12755)). Contributed by @richvdh.

## 🐛 Bug Fixes

* Add a modernizr check for WebAssembly support ([#27776](element-hq/element-web#27776)). Contributed by @dbkr.
* Test for lack of WebAssembly support ([#12792](matrix-org/matrix-react-sdk#12792)). Contributed by @dbkr.
* Fix stray 'account' heading ([#12791](matrix-org/matrix-react-sdk#12791)). Contributed by @dbkr.
* Add test for the unsupported browser screen ([#12787](matrix-org/matrix-react-sdk#12787)). Contributed by @dbkr.
* Fix HTML export test ([#12778](matrix-org/matrix-react-sdk#12778)). Contributed by @dbkr.
* Fix HTML export missing a bunch of Compound variables ([#12774](matrix-org/matrix-react-sdk#12774)). Contributed by @t3chguy.
* Fix inability to change accent colour consistently in custom theming ([#12772](matrix-org/matrix-react-sdk#12772)). Contributed by @t3chguy.
* Fix edge case of landing on 3pid email link with registration disabled ([#12771](matrix-org/matrix-react-sdk#12771)). Contributed by @t3chguy.



Changes in [1.11.71](https://github.com/element-hq/element-web/releases/tag/v1.11.71) (2024-07-16)
==================================================================================================
## ✨ Features

* Add Modernizr rule for Intl.Segmenter ([#27677](element-hq/element-web#27677)). Contributed by @t3chguy.
* Add tabs to the right panel ([#12672](matrix-org/matrix-react-sdk#12672)). Contributed by @MidhunSureshR.
* Promote new room header from labs to Beta ([#12739](matrix-org/matrix-react-sdk#12739)). Contributed by @t3chguy.
* Redesign room search interface ([#12677](matrix-org/matrix-react-sdk#12677)). Contributed by @t3chguy.
* Move language settings to 'preferences' ([#12723](matrix-org/matrix-react-sdk#12723)). Contributed by @dbkr.
* New layout selector ui in user settings ([#12676](matrix-org/matrix-react-sdk#12676)). Contributed by @florianduros.
* Prevent Element appearing in system media controls  ([#10995](matrix-org/matrix-react-sdk#10995)). Contributed by @SuperKenVery.
* Move the account management button ([#12663](matrix-org/matrix-react-sdk#12663)). Contributed by @dbkr.
* Disable profile controls if the HS doesn't allow them to be set ([#12652](matrix-org/matrix-react-sdk#12652)). Contributed by @dbkr.
* New theme ui in user settings ([#12576](matrix-org/matrix-react-sdk#12576)). Contributed by @florianduros.
* Adjust room header hover transition from 300ms to 200ms ([#12703](matrix-org/matrix-react-sdk#12703)). Contributed by @t3chguy.
* Split out email \& phone number settings to separate components \& move discovery to privacy tab ([#12670](matrix-org/matrix-react-sdk#12670)). Contributed by @dbkr.

## 🐛 Bug Fixes

* Ensure we do not load matrix-react-sdk is a manner which can white-screen ([#27685](element-hq/element-web#27685)). Contributed by @t3chguy.
* Fix incoming call toast crash due to audio refactor ([#12737](matrix-org/matrix-react-sdk#12737)). Contributed by @t3chguy.
* Improve new room header accessibility ([#12725](matrix-org/matrix-react-sdk#12725)). Contributed by @t3chguy.
* Fix closing all modals ([#12728](matrix-org/matrix-react-sdk#12728)). Contributed by @dbkr.
* Fix close button on forgot password flow ([#12732](matrix-org/matrix-react-sdk#12732)). Contributed by @dbkr.
* Don't consider textual characters to be emoji ([#12582](matrix-org/matrix-react-sdk#12582)). Contributed by @robintown.
* Clear autocomplete input on selection accept ([#12709](matrix-org/matrix-react-sdk#12709)). Contributed by @dbkr.
* Fix `Match system theme` toggle ([#12719](matrix-org/matrix-react-sdk#12719)). Contributed by @florianduros.



Changes in [1.11.70](https://github.com/element-hq/element-web/releases/tag/v1.11.70) (2024-07-08)
==================================================================================================
## ✨ Features

* Add SSO redirect option for login page ([#27576](element-hq/element-web#27576)). Contributed by @bartvdbraak.
* Use stable endpoints for MSC3916 ([#27558](element-hq/element-web#27558)). Contributed by @turt2live.
* Switch to Rust crypto stack for all logins ([#12630](matrix-org/matrix-react-sdk#12630)). Contributed by @richvdh.
* Hide voip buttons in group rooms in environments with widgets disabled ([#12664](matrix-org/matrix-react-sdk#12664)). Contributed by @t3chguy.
* Minor tweaks to UserSettings dialog ([#12651](matrix-org/matrix-react-sdk#12651)). Contributed by @florianduros.
* Hide voice call button when redundant ([#12639](matrix-org/matrix-react-sdk#12639)). Contributed by @t3chguy.
* Improve accessibility of the room summary card ([#12586](matrix-org/matrix-react-sdk#12586)). Contributed by @t3chguy.
* Show tooltips on narrow tabbed views ([#12624](matrix-org/matrix-react-sdk#12624)). Contributed by @dbkr.
* Update gfm.css to github-markdown-css ([#12613](matrix-org/matrix-react-sdk#12613)). Contributed by @t3chguy.
* Cache e2eStatus to avoid concerning unencrypted flicker when changing rooms ([#12606](matrix-org/matrix-react-sdk#12606)). Contributed by @t3chguy.
* Tweak copy for user verification toast ([#12605](matrix-org/matrix-react-sdk#12605)). Contributed by @t3chguy.
* Support s tags for strikethrough for Matrix v1.10 ([#12604](matrix-org/matrix-react-sdk#12604)). Contributed by @t3chguy.

## 🐛 Bug Fixes

* Fix "Unable to restore session" error ([#4299](matrix-org/matrix-js-sdk#4299)).
* Fix error when sending encrypted messages in large rooms ([#4297](matrix-org/matrix-js-sdk#4297)).
* Remove redundant copy in deactive uia modal ([#12668](matrix-org/matrix-react-sdk#12668)). Contributed by @t3chguy.
* Fix high contrast theme in settings ([#12649](matrix-org/matrix-react-sdk#12649)). Contributed by @florianduros.
* Fix background on live location sharing footer ([#12629](matrix-org/matrix-react-sdk#12629)). Contributed by @t3chguy.
* Remove outdated iframe sandbox attribute ([#12633](matrix-org/matrix-react-sdk#12633)). Contributed by @t3chguy.
* Remove stray setState which caused encryption state shields to flicker ([#12632](matrix-org/matrix-react-sdk#12632)). Contributed by @t3chguy.
* Fix stray background colour on markdown body ([#12628](matrix-org/matrix-react-sdk#12628)). Contributed by @t3chguy.
* Fix widgets not being cleaned up correctly. ([#12616](matrix-org/matrix-react-sdk#12616)). Contributed by @toger5.
* Add in-progress view to display name EditInPlace ([#12609](matrix-org/matrix-react-sdk#12609)). Contributed by @dbkr.
* Fix config override of other settings levels ([#12593](matrix-org/matrix-react-sdk#12593)). Contributed by @langleyd.
* Don't show 'saved' on display name save error ([#12600](matrix-org/matrix-react-sdk#12600)). Contributed by @dbkr.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@t3chguy t3chguy t3chguy approved these changes

@dbkr dbkr Awaiting requested review from dbkr dbkr is a code owner automatically assigned from matrix-org/element-web-reviewers

@richvdh richvdh Awaiting requested review from richvdh richvdh is a code owner automatically assigned from matrix-org/element-web-reviewers

Assignees

@langleyd langleyd

Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot enable labs flags which are supposed to be force-enabled via config.json
2 participants
@langleyd @t3chguy