1907779 - Back navigation gets broken on HistoryFragment when device theme has changed.

Status: RESOLVED FIXED

(Firefox for Android :: History, defect, P2)

Description

[Kaya [:kaya]]

Steps to reproduce

1. Navigate to History from Settings
2. Go to device settings and change device theme (does not matter if it is to dark or to light)
3. Come back to app and back press on History fragment.

Expected behavior

After back pressing on History fragment, we should navigate back to Home page.

Actual behavior

Back pressing causes a bug in the back stack entry, ending up taking app to the background. Because of the broken back stack, the user will never be able to go back to home page and continue browsing, only way would be killing&restarting the app.

Comment 1

[Kaya [:kaya]]

Linking a potential regression cause. Needs confirmation.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1842250

Comment 3

[Kaya [:kaya]]

I have a quick fix for this:

diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt
index 5de7644db371a..d0ac86bc93cd8 100644
--- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt
+++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt
@@ -368,8 +368,12 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler,
 
     override fun onBackPressed(): Boolean {
         // The state needs to be updated accordingly if Edit mode is active
-        historyStore.dispatch(HistoryFragmentAction.BackPressed)
-        return true
+        return if (historyStore.state.mode is HistoryFragmentState.Mode.Editing) {
+            historyStore.dispatch(HistoryFragmentAction.BackPressed)
+            true
+        } else {
+            false
+        }
     }
 
     override fun onDestroyView() {

But it may also be related to the order of backstack entries when app is resumed after the device theme has changed. It is worth checking out to have a better understanding of the root cause. I checked the backstack entries over adb and looks like the HomeFragment and HistoryFragment's order get switched. I'll unassign and clear the regression tags for now and come back when I'm not busy if this is not picked by then. Clearing severity as well.

Comment 4

[Rahul Sainani [:rsainani]]

While looking into it, also found that on theme change, other actions like opening recentlly closed and opening an item does not work. Also, trying to delete an item results in a crash.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

:matt-tighe, since you are the author of the regressor, bug 1842250, could you take a look?

For more information, please visit BugBot documentation.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1842250

Comment 7

[Nicholas Poon (redirect NI to [e-mail]) [:Nick]]

As discussed with Rahul offline, this issue is quite impactful as it not only impacts the back navigation but the entire history screen. After coming back to the app after a device theme change (step 3 in the STR), the whole history page practically does not work, with the exception only being share. To fully address the issue, it is best to do a history refactor. However, before that happens, there is no obvious way to address all of these problems. Thus, I have opened separate issues for every broken element in history after this theme change. I have linked them in the "See Also" section of this bug.

As a result, I think kaya's suggestion is indeed correct and can be used to close this ticket. All other issues with history can be fixed in the follow up bugs

Comment 8

[Nicholas Poon (redirect NI to [e-mail]) [:Nick]]

Attachment: Bug 1907779 - Fix back navigation on `HistoryFragment` when device theme has changed

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d4e29a519c0d
Fix back navigation on `HistoryFragment` when device theme has changed r=android-reviewers,rsainani

Comment 10

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/d4e29a519c0d

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:npoon, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox132 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Nicholas Poon (redirect NI to [e-mail]) [:Nick]]

Requesting QA to verify that the bug has been resolved before requesting uplift

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Just for clarity, we're building the final Beta of the cycle on Friday before next week's RC. If the plan is to uplift this, we'll need the request pretty soon.

Comment 14

[Delia Pop (Android QA)]

Attachment: media_20241018_092051_6390995698119242321.mp4

Verified as fixed in the latest Nightly 13.0a1 from 10/18 with Google Pixel 8 Pro (Android 14), OPPO A15s (Android 10)
and Lenovo Yoga Tab 11 (Android 12).

Comment 15

[Nicholas Poon (redirect NI to [e-mail]) [:Nick]]

Thanks for verifying, Delia! And thank you Ryan for the information! In that case, let's just keep this fix in 133 and let it ride the trains normally until release. This issue has been present for quite some time now so I think it's fine to leave it. It was also important to get verification before uplifting to make sure that history didn't break after the changes made.