1923650 - AppState Browsing mode is not consistent

Status: RESOLVED FIXED

(Firefox for Android :: General, task, P3)

Description

[Rahul Sainani [:rsainani]]

Users of AppState.BrowsingMode won't always get the latest browsing mode. This works fine when on the HomeFragment but not otherwise. Discussion from this patch .

Comment 1

[Noah Bond [:007]]

I wonder if, for the short term, we can add the following line:

components.appStore.dispatch(AppAction.ModeChange(mode = newMode))

to this section HomeActivity where Browsing Mode Manager is set up, so the AppStore is always updated when the manager is updated.

Comment 2

[Rahul Sainani [:rsainani]]

That's a good idea! We should give this approach a try.

Comment 3

[Mike a [:mavduevskiy]]

Attachment: Bug 1923650 - Part 1: Update AppState.mode every time BrowsingModeManager is updated

Comment 4

[Mike a [:mavduevskiy]]

Attachment: Bug 1923650 - Part 2: Discourage usage of ModeChange

An attempt to force users to update the mode from BrowserModeManager only, to reduce the chance of them going async.

Comment 5

[Mike a [:mavduevskiy]]

Attachment: Bug 1923650 - Part 3: Remove browser mode update action dispatches outsidei of BrowsingModeManager

Comment 6

[Mike a [:mavduevskiy]]

Attachment: Bug 1923650 - Part 4: Remove the toggleBrowsingMode helper function from tabstrip

It was used to update the state in two places; now that we need only update it once, the function could be removed.

Comment 7

[Mike a [:mavduevskiy]]

For some extra context:

1. the patch follows in implementation the original [[ https://github.com/mozilla-mobile/firefox-android/pull/5502/files#diff-e9a193a65990d2de50218c9d11e0b45ad43afad7ef28fd81e47be3591fc4cc84R50 | patch ]] of :matt-tighe;
2. from a discussion with Matt today: though not possible to restrict, we wanted to descourage usage of ModeChange outside of the Manager by changing the name of the ModeChange action to hint that it should only be called from the manager.

Comment 8

[Mike a [:mavduevskiy]]

Attachment: Bug 1923650 - Part 5: Don't update homepage composable on appstate mode change

Currently, changing a theme on homepage relies on ThemeManager restarting the activity, and the HomeFragment being created with the updated theme and content at the same time.

Allowing HomeFragment to be updated by the appstate mode change led to the content being updated, but theme updated was lagging behind and was only applied after the fragment was recreated, leading to a visual glitch.

Comment 9

[Mike a [:mavduevskiy]]

Attachment: Bug 1923650 - Part 6: Refactor DefaultBrowserModeManager to avoid spies in tests

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f21c5c815073
https://hg.mozilla.org/integration/autoland/rev/b7952c3b476d
Part 1: Update AppState.mode every time BrowsingModeManager is updated r=android-reviewers,007
https://github.com/mozilla-firefox/firefox/commit/11227b16a988
https://hg.mozilla.org/integration/autoland/rev/3bbb7778c37a
Part 2: Discourage usage of ModeChange r=android-reviewers,007
https://github.com/mozilla-firefox/firefox/commit/d41604b58222
https://hg.mozilla.org/integration/autoland/rev/3a5e4b77ca6f
Part 3: Remove browser mode update action dispatches outsidei of BrowsingModeManager r=android-reviewers,007
https://github.com/mozilla-firefox/firefox/commit/062611bfc08c
https://hg.mozilla.org/integration/autoland/rev/e2198b52f790
Part 4: Remove the toggleBrowsingMode helper function from tabstrip r=android-reviewers,007
https://github.com/mozilla-firefox/firefox/commit/606b3d18374f
https://hg.mozilla.org/integration/autoland/rev/35150c30773d
Part 5: Don't update homepage composable on appstate mode change r=android-reviewers,matt-tighe
https://github.com/mozilla-firefox/firefox/commit/c5395bfbf87c
https://hg.mozilla.org/integration/autoland/rev/4d6a60329173
Part 6: Refactor DefaultBrowserModeManager to avoid spies in tests r=android-reviewers,007

Comment 11

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/b7952c3b476d
https://hg.mozilla.org/mozilla-central/rev/3bbb7778c37a
https://hg.mozilla.org/mozilla-central/rev/3a5e4b77ca6f
https://hg.mozilla.org/mozilla-central/rev/e2198b52f790
https://hg.mozilla.org/mozilla-central/rev/35150c30773d
https://hg.mozilla.org/mozilla-central/rev/4d6a60329173