Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage #11902

mgcm · 2023-11-17T16:30:04Z

EW handles language setting on init and saves it in localStorage, using React SDK's SettingsStore.

When in a delegated auth environment, the success handler will clear the localStorage, including the previously set local settings language value.

This results in not having that language value available in localStorage right after starting a new user session within such an environment.

This PR fixes that by checking if a language value is set in the SettingsStore and keeping it after clearing the localStorage.

Checklist

Tests written for new code (and old code if feasible)
Linter and other CI checks pass
Sign-off given on the changes (see CONTRIBUTING.md)

Type: defect

Here's what your changelog entry will look like:

🐛 Bug Fixes

Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage (#11902). Contributed by @mgcm.

Signed-off-by: Milton Moura <miltonmoura@gmail.com>

src/Lifecycle.ts

Signed-off-by: Milton Moura <miltonmoura@gmail.com>

t3chguy

The way this PR stands, it'll bake the default (config.json) language into localStorage at the time the code runs, this is not expected. The intent is to preserve the setting, if one was set, but its currently "preserving" the default into a Device-level setting.

mgcm · 2023-11-21T16:49:20Z

@t3chguy Would you agree to check directly within mx_local_settings for the existence of the language key / value instead ?

t3chguy · 2023-11-21T16:51:03Z

@mgcm no, take a look at https://github.com/vector-im/element-web/blob/0040181489feda0e5d70becdea8585ac5a344d99/src/vector/init.tsx#L122

mgcm · 2023-11-21T17:20:36Z

@t3chguy changed to get the value at the device level exclusively

t3chguy · 2023-11-21T17:25:55Z

@mgcm I'm not seeing any new commits from you, once you wish for a re-review please request one top right

Signed-off-by: Milton Moura <miltonmoura@gmail.com>

t3chguy

This now looks like it does the right thing, any chance of tests?

…ssfull delegated authentication flow Signed-off-by: Milton Moura <miltonmoura@gmail.com>

mgcm · 2023-11-24T07:21:20Z

I struggled a bit to find the most appropriate place for adding the test and settled with the MatrixChat-test suite because it's where auth flows are being tested.

I also moved the OIDC tests up due to having the new test case failing when it runs after the "with an existing session / logout", which seem to mess up the test state (due to the actual dispatch logout action).

Would appreciate some feedback :)

t3chguy · 2023-11-24T08:36:44Z

test/components/structures/MatrixChat-test.tsx

+            it("should persist device language when available", async () => {
+                localStorage.setItem("mx_local_settings", localSettings);
+
+                jest.spyOn(Lifecycle, "attemptDelegatedAuthLogin");
+
+                getComponent({ realQueryParams });
+                await flushPromises();
+
+                expect(Lifecycle.attemptDelegatedAuthLogin).toHaveBeenCalled();
+                expect(localStorage.getItem("mx_local_settings")).toEqual(localSettings);
+            });


This tests the happy path - great - it'd also be good to test the path which the original solution did not handle - not setting the language if there wasn't one set to begin with, please.

Added the unhappy path and slight refactor. Thanks!

…ad of going directly to localStorage Signed-off-by: Milton Moura <miltonmoura@gmail.com>

Signed-off-by: Milton Moura <miltonmoura@gmail.com>

t3chguy

Looks good to me, thanks.

The type fail is unrelated and has a PR pending review to fix

t3chguy · 2023-11-24T12:06:35Z

@mgcm if you could pull in latest develop that should fix CI

@RiotRobot

Changes in [1.11.55](https://github.com/element-hq/element-web/releases/tag/v1.11.55) (2024-01-19) ================================================================================================== ## ✨ Features * Broaden support for matrix spec versions ([#12159](matrix-org/matrix-react-sdk#12159)). Contributed by @RiotRobot. ## 🐛 Bug Fixes * Fixed shield alignment on message Input ([#12155](matrix-org/matrix-react-sdk#12155)). Contributed by @RiotRobot. Changes in [1.11.54](https://github.com/element-hq/element-web/releases/tag/v1.11.54) (2024-01-16) ================================================================================================== ## 🐛 Bug Fixes * Fix CSS stacking context order determinism ([#26840](element-hq/element-web#26840)). Contributed by @t3chguy. ## ✨ Features * Accessibility improvements around aria-labels and tooltips ([#12062](matrix-org/matrix-react-sdk#12062)). Contributed by @t3chguy. * Add RoomKnocksBar to RoomHeader ([#12077](matrix-org/matrix-react-sdk#12077)). Contributed by @charlynguyen. * Adjust tooltip side for DecoratedRoomAvatar to not obscure room name ([#12079](matrix-org/matrix-react-sdk#12079)). Contributed by @t3chguy. * Iterate landmarks around the app in order to improve a11y ([#12064](matrix-org/matrix-react-sdk#12064)). Contributed by @t3chguy. * Update element call embedding UI ([#12056](matrix-org/matrix-react-sdk#12056)). Contributed by @toger5. * Use Compound tooltips instead of homegrown in TextWithTooltip \& InfoTooltip ([#12052](matrix-org/matrix-react-sdk#12052)). Contributed by @t3chguy. ## 🐛 Bug Fixes * Fix regression around CSS stacking contexts and PIP widgets ([#12094](matrix-org/matrix-react-sdk#12094)). Contributed by @t3chguy. * Fix Identity Server terms accepting not working as expected ([#12109](matrix-org/matrix-react-sdk#12109)). Contributed by @t3chguy. * fix: microphone and camera dropdown doesn't work In legacy call ([#12105](matrix-org/matrix-react-sdk#12105)). Contributed by @muratersin. * Revert "Set up key backup using non-deprecated APIs (#12005)" ([#12102](matrix-org/matrix-react-sdk#12102)). Contributed by @BillCarsonFr. * Fix regression around read receipt animation from refs changes ([#12100](matrix-org/matrix-react-sdk#12100)). Contributed by @t3chguy. * Added meaning full error message based on platform ([#12074](matrix-org/matrix-react-sdk#12074)). Contributed by @Pankaj-SinghR. * Fix editing event from search room view ([#11992](matrix-org/matrix-react-sdk#11992)). Contributed by @t3chguy. * Fix timeline position when moving to a room and coming back ([#12055](matrix-org/matrix-react-sdk#12055)). Contributed by @florianduros. * Fix threaded reply playwright tests ([#12070](matrix-org/matrix-react-sdk#12070)). Contributed by @dbkr. * Element-R: fix repeated requests to enter 4S key during cross-signing reset ([#12059](matrix-org/matrix-react-sdk#12059)). Contributed by @richvdh. * Fix position of thumbnail in room timeline ([#12016](matrix-org/matrix-react-sdk#12016)). Contributed by @anoopw3bdev. Changes in [1.11.53](https://github.com/element-hq/element-web/releases/tag/v1.11.53) (2024-01-04) ================================================================================================== ## 🐛 Bug Fixes * Fix a fresh login creating a new key backup ([#12106](matrix-org/matrix-react-sdk#12106)). Changes in [1.11.52](https://github.com/element-hq/element-web/releases/tag/v1.11.52) (2023-12-19) ================================================================================================== ## ✨ Features * Keep more recent rageshake logs ([#12003](matrix-org/matrix-react-sdk#12003)). Contributed by @richvdh. ## 🐛 Bug Fixes * Fix bug which prevented correct clean up of rageshake store ([#12002](matrix-org/matrix-react-sdk#12002)). Contributed by @richvdh. * Set up key backup using non-deprecated APIs ([#12005](matrix-org/matrix-react-sdk#12005)). Contributed by @andybalaam. * Fix notifications appearing for old events ([#3946](matrix-org/matrix-js-sdk#3946)). Contributed by @dbkr. * Prevent phantom notifications from events not in a room's timeline ([#3942](matrix-org/matrix-js-sdk#3942)). Contributed by @dbkr. Changes in [1.11.51](https://github.com/vector-im/element-web/releases/tag/v1.11.51) (2023-12-05) ================================================================================================= ## ✨ Features * Improve debian package and docs ([#26618](element-hq/element-web#26618)). Contributed by @t3chguy. ## 🦖 Deprecations * Remove Quote from MessageContextMenu as it is unsupported by WYSIWYG ([#11914](matrix-org/matrix-react-sdk#11914)). Contributed by @t3chguy. ## ✨ Features * Always allow call.member events on new rooms ([#11948](matrix-org/matrix-react-sdk#11948)). Contributed by @toger5. * Right panel: view third party invite info without clearing history ([#11934](matrix-org/matrix-react-sdk#11934)). Contributed by @kerryarchibald. * Allow switching to system emoji font ([#11925](matrix-org/matrix-react-sdk#11925)). Contributed by @t3chguy. * Update open in other tab message ([#11916](matrix-org/matrix-react-sdk#11916)). Contributed by @weeman1337. * Add menu for legacy and element call in 1:1 rooms ([#11910](matrix-org/matrix-react-sdk#11910)). Contributed by @toger5. * Add ringing for matrixRTC ([#11870](matrix-org/matrix-react-sdk#11870)). Contributed by @toger5. ## 🐛 Bug Fixes * Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage ([#11902](matrix-org/matrix-react-sdk#11902)). Contributed by @mgcm. * Fix misunderstanding of functional members ([#11918](matrix-org/matrix-react-sdk#11918)). Contributed by @toger5. * Fix: Video Room Chat Header Button Removed ([#11911](matrix-org/matrix-react-sdk#11911)). Contributed by @kerryarchibald. * Fix "not attempting encryption" warning ([#11899](matrix-org/matrix-react-sdk#11899)). Contributed by @richvdh. Changes in [1.11.50](https://github.com/vector-im/element-web/releases/tag/v1.11.50) (2023-11-21) ================================================================================================= ## ✨ Features * Ship element-web as a debian package ([#26533](element-hq/element-web#26533)). Contributed by @t3chguy. * Update room summary card header ([#11823](matrix-org/matrix-react-sdk#11823)). Contributed by @germain-gg. * Add feature flag for disabling encryption in Element Call ([#11837](matrix-org/matrix-react-sdk#11837)). Contributed by @toger5. * Adapt the rendering of extra icons in the room header ([#11835](matrix-org/matrix-react-sdk#11835)). Contributed by @charlynguyen. * Implement new unreachable state and fix broken string ref ([#11748](matrix-org/matrix-react-sdk#11748)). Contributed by @MidhunSureshR. * Allow adding extra icons to the room header ([#11799](matrix-org/matrix-react-sdk#11799)). Contributed by @charlynguyen. ## 🐛 Bug Fixes * Room header: do not collapse avatar or facepile ([#11866](matrix-org/matrix-react-sdk#11866)). Contributed by @kerryarchibald. * New right panel: fix button alignment in memberlist ([#11861](matrix-org/matrix-react-sdk#11861)). Contributed by @kerryarchibald. * Use the correct video call icon variant ([#11859](matrix-org/matrix-react-sdk#11859)). Contributed by @robintown. * fix broken warning icon ([#11862](matrix-org/matrix-react-sdk#11862)). Contributed by @ara4n. * Fix rightpanel hiding scrollbar ([#11831](matrix-org/matrix-react-sdk#11831)). Contributed by @kerryarchibald. * Switch to updating presence via /sync calls instead of PUT /presence ([#11824](matrix-org/matrix-react-sdk#11824)). Contributed by @t3chguy. Changes in [1.11.49](https://github.com/vector-im/element-web/releases/tag/v1.11.49) (2023-11-13) ================================================================================================= ## ✨ Features * Ship element-web as a debian package ([\#26533](element-hq/element-web#26533)). Fixes #2777. ## 🐛 Bug Fixes * Ensure `setUserCreator` is called when a store is assigned ([\#3867](matrix-org/matrix-js-sdk#3867)). Fixes element-hq/element-web#26520. Contributed by @MidhunSureshR. Changes in [1.11.48](https://github.com/vector-im/element-web/releases/tag/v1.11.48) (2023-11-07) ================================================================================================= ## ✨ Features * Correctly fill window.matrixChat even when a Wrapper module is active ([\#26395](element-hq/element-web#26395)). Contributed by @dhenneke. * Knock on a ask-to-join room if a module wants to join the room when navigating to a room ([\#11787](matrix-org/matrix-react-sdk#11787)). Contributed by @dhenneke. * Element-R: Include crypto info in sentry ([\#11798](matrix-org/matrix-react-sdk#11798)). Contributed by @florianduros. * Element-R: Include crypto info in rageshake ([\#11797](matrix-org/matrix-react-sdk#11797)). Contributed by @florianduros. * Element-R: Add current version of the rust-sdk and vodozemac ([\#11785](matrix-org/matrix-react-sdk#11785)). Contributed by @florianduros. * Fix unfederated invite dialog ([\#9618](matrix-org/matrix-react-sdk#9618)). Fixes element-hq/element-meta#1466 and #22102. Contributed by @owi92. * New right panel visual language ([\#11664](matrix-org/matrix-react-sdk#11664)). * OIDC: add friendly errors ([\#11184](matrix-org/matrix-react-sdk#11184)). Fixes #25665. Contributed by @kerryarchibald. ## 🐛 Bug Fixes * Fix rightpanel hiding scrollbar ([\#11831](matrix-org/matrix-react-sdk#11831)). Contributed by @kerryarchibald. * Fix multi-tab session lock on Firefox not being cleared ([\#11800](matrix-org/matrix-react-sdk#11800)). Fixes #26165. Contributed by @ManuelHu. * Deserialise spoilers back into slash command form ([\#11805](matrix-org/matrix-react-sdk#11805)). Fixes #26344. * Fix Incorrect message scaling for verification request ([\#11793](matrix-org/matrix-react-sdk#11793)). Fixes #24304. Contributed by @capGoblin. * Fix: Unable to restore a soft-logged-out session established via SSO ([\#11794](matrix-org/matrix-react-sdk#11794)). Fixes #25957. Contributed by @kerryarchibald. * Use configurable github issue links more consistently ([\#11796](matrix-org/matrix-react-sdk#11796)). * Fix io.element.late_event received_ts vs received_at ([\#11789](matrix-org/matrix-react-sdk#11789)). * Make invitation dialog scrollable when infos are too long ([\#11753](matrix-org/matrix-react-sdk#11753)). Contributed by @nurjinjafar. * Fix spoiler text-align ([\#11790](matrix-org/matrix-react-sdk#11790)). Contributed by @ajbura. * Fix: Right panel keeps showing chat when unmaximizing widget. ([\#11697](matrix-org/matrix-react-sdk#11697)). Fixes #26265. Contributed by @manancodes. * Fix margin of invite to room button ([\#11780](matrix-org/matrix-react-sdk#11780)). Fixes #26410. * Update base64 import ([\#11784](matrix-org/matrix-react-sdk#11784)). * Set max size for Element logo in search warning ([\#11779](matrix-org/matrix-react-sdk#11779)). Fixes #26408. * Fix: emoji size in room header topic, remove obsolete emoji style ([\#11757](matrix-org/matrix-react-sdk#11757)). Fixes #26326. Contributed by @kerryarchibald. * Fix: Bubble layout design is broken ([\#11763](matrix-org/matrix-react-sdk#11763)). Fixes #25818. Contributed by @manancodes.

Do not remove the language when clearing local storage

d0d4f7c

Signed-off-by: Milton Moura <miltonmoura@gmail.com>

github-actions bot added Z-Community-PR Issue is solved by a community member's PR T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems labels Nov 17, 2023

mgcm changed the title ~~Keep the language code in localStorage when it has been previosuly set by EW after a successful delegated authentication flow~~ Keep language in localStorage when it has been previosuly set by EW after a successful delegated authentication flow Nov 17, 2023

mgcm marked this pull request as ready for review November 17, 2023 16:33

mgcm requested a review from a team as a code owner November 17, 2023 16:33

mgcm requested review from t3chguy and kerryarchibald November 17, 2023 16:33

github-actions bot deployed to Netlify November 17, 2023 16:33 View deployment

t3chguy reviewed Nov 20, 2023

View reviewed changes

src/Lifecycle.ts Outdated Show resolved Hide resolved

mgcm added 2 commits November 21, 2023 13:34

Revised comment on getting the defined language from settings

5c4b7fa

Signed-off-by: Milton Moura <miltonmoura@gmail.com>

Merge branch 'develop' into mgcm/fix/oidc-auth-clear-storage

4651389

github-actions bot deployed to Netlify November 21, 2023 14:40 View deployment

t3chguy suggested changes Nov 21, 2023

View reviewed changes

mgcm changed the title ~~Keep language in localStorage when it has been previosuly set by EW after a successful delegated authentication flow~~ Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage Nov 21, 2023

Explicitly checking for language set at the device level

95af2a9

Signed-off-by: Milton Moura <miltonmoura@gmail.com>

github-actions bot deployed to Netlify November 21, 2023 17:32 View deployment

mgcm requested a review from t3chguy November 21, 2023 20:28

Merge branch 'develop' into mgcm/fix/oidc-auth-clear-storage

b8f2c19

github-actions bot deployed to Netlify November 22, 2023 00:12 View deployment

t3chguy reviewed Nov 22, 2023

View reviewed changes

mgcm added 2 commits November 24, 2023 06:12

Merge branch 'develop' into mgcm/fix/oidc-auth-clear-storage

b1fb9c5

Add test that checks if device language setting is kept after a succe…

4d62a10

…ssfull delegated authentication flow Signed-off-by: Milton Moura <miltonmoura@gmail.com>

github-actions bot deployed to Netlify November 24, 2023 07:18 View deployment

t3chguy reviewed Nov 24, 2023

View reviewed changes

mgcm added 2 commits November 24, 2023 09:35

Adds test for unhappy path and adjusts to use the SettingsStore inste…

5da0146

…ad of going directly to localStorage Signed-off-by: Milton Moura <miltonmoura@gmail.com>

Removing unnecessary variable after test refactor

3fca38b

Signed-off-by: Milton Moura <miltonmoura@gmail.com>

github-actions bot deployed to Netlify November 24, 2023 11:02 View deployment

mgcm requested a review from t3chguy November 24, 2023 11:15

t3chguy approved these changes Nov 24, 2023

View reviewed changes

t3chguy enabled auto-merge November 24, 2023 11:20

Merge branch 'develop' into mgcm/fix/oidc-auth-clear-storage

d38972e

github-actions bot deployed to Netlify November 24, 2023 15:31 View deployment

t3chguy added this pull request to the merge queue Nov 24, 2023

Merged via the queue into matrix-org:develop with commit 76b7aa2 Nov 24, 2023
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage #11902

Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage #11902

mgcm commented Nov 17, 2023 •

edited by github-actions bot

Loading

t3chguy left a comment •

edited

Loading

mgcm commented Nov 21, 2023

t3chguy commented Nov 21, 2023

mgcm commented Nov 21, 2023

t3chguy commented Nov 21, 2023

t3chguy left a comment

mgcm commented Nov 24, 2023

t3chguy Nov 24, 2023

mgcm Nov 24, 2023

t3chguy left a comment

t3chguy commented Nov 24, 2023

Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage #11902

Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage #11902

Conversation

mgcm commented Nov 17, 2023 • edited by github-actions bot Loading

Checklist

🐛 Bug Fixes

t3chguy left a comment • edited Loading

Choose a reason for hiding this comment

mgcm commented Nov 21, 2023

t3chguy commented Nov 21, 2023

mgcm commented Nov 21, 2023

t3chguy commented Nov 21, 2023

t3chguy left a comment

Choose a reason for hiding this comment

mgcm commented Nov 24, 2023

t3chguy Nov 24, 2023

Choose a reason for hiding this comment

mgcm Nov 24, 2023

Choose a reason for hiding this comment

t3chguy left a comment

Choose a reason for hiding this comment

t3chguy commented Nov 24, 2023

mgcm commented Nov 17, 2023 •

edited by github-actions bot

Loading

t3chguy left a comment •

edited

Loading