1936359 - Undo snackbar is shown indefinitely preventing interactions with the toolbar

Status: VERIFIED FIXED

(Fenix :: History, defect)

Description

[Petru-Mugurel Lingurar [:petru]]

Attachment: ModalSnackbar.mp4

Steps to reproduce:

• while browsing open the history screen from the hamburger menu
• delete a history entry
• tap back to go to browsing while the snackbar is showing

Expected result:

• an undo snackbar appears that informs the user about the successful deletion and allows to undo the operation
• this snackbar disappears after a few seconds / allows the user to swipe it away.

Actual result:

• an undo snackbar appears that informs the user about the successful deletion and allows to undo the operation
• this snackbar stays indefinitely on the screen blocking other interactions
• tapping on "Undo" dismisses the snackbar but upon checking in history the operation has not been undone.

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Attachment: SnackbarInRelease133.mp4

How the snackbar for the same scenario looks like in Release 133 ^^.

Comment 2

[Petru-Mugurel Lingurar [:petru]]

[Tracking Requested - why for this release]:

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Think this was caused by bug 1924196.
Nick can you check?

Comment 4

[Nicholas Poon [:Nick]]

Thanks Petru! I took a look into it and I think you're right - this is a regression from the bug that you mentioned

Comment 5

[Nicholas Poon [:Nick]]

Some things that I noticed in the investigation

• When calling allowUndo, changing from lifecycleScope back to CoroutineScope(IO) prevents the indefinite duration. I noticed that the indefinite duration only occurs when deleting a history item and navigation away from the history fragment. When deleting a history item and staying on the history fragment, the snackbar will dismiss as expected. I think the issue is that when we navigate away from the fragment, the coroutine ends (due to it being bound to the fragment's lifecycle). Then, the launched coroutine for the delay gets cancelled and the dismiss never gets called [searchfox].
• After doing this, there was another issue. Now, when navigating away from the history fragment on history item delete, the snackbar was being dismissed as expected. However, the app would crash immediately afterwards. This is the result of calling requireContext in the delete function in history. The function was not behaving as expected because when navigated away from history fragment, there is no fragment for the context to be attached to anymore.
  • This suggests that delete is only being called when the snackbar is dismissed, which is different from 133. It should be called when I hit the delete button. I can confirm that this is the case with this STR: I delete a history item, navigate away from history fragment, navigate back to history fragment. I must be able to complete all these steps before the snackbar dismisses.
  • After doing this, I see that the history item is still there. Once the snackbar dismisses, I navigate away and back to the history fragment again and the item is no longer there.

Comment 6

[Nicholas Poon [:Nick]]

Attachment: Bug 1936359 - Fix history undo snackbar on fragment change

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/183994e76123
Fix history undo snackbar on fragment change r=android-reviewers,tthibaud

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/183994e76123

Comment 10

[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-firefox134 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[Nicholas Poon [:Nick]]

Ideally, after QA verification, I would like to have this uplifted but I'm aware that it's too late to uplift to beta 134 now. After the merge day, will there be a time where we can uplift beta 135 to release 134?

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Yes, it could still go into one of this cycle's planned Android dot releases. Feel free to add a mozilla-release approval request whenever you're ready.

Comment 13

[Mira Lobontiu (Android QA)]

Verified as fixed on the Fenix Nightly 135.0a1 from 1/2 with a Samsung Galaxy Tab S9 Ultra (Android 14), Google Pixel 6 (Android 15), and OnePlus 5 (Android 10).

Comment 14

[Nicholas Poon [:Nick]]

Comment on attachment 9442800 [details]
Bug 1936359 - Fix history undo snackbar on fragment change

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: Snackbar blocks toolbar indefinitely (until app restart) on history delete
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): I've already tested it locally and QE has verified the fix as well
• String changes made/needed:
• Is Android affected?: Yes

Comment 15

[Nicholas Poon [:Nick]]

Hey Ryan,
I was wondering if we have any update on the uplift request. It would be nice to get this uplifted to Monday's dot release

Comment 16

[Ryan VanderMeulen [:RyanVM]]

The deadline for uplift requests is today. They'll be reviewed on Monday ahead of the planned dot release build. We don't land them ahead of time so we can more easily react in the event of an unplanned dot release situation.

Also, please note that Pascal is the Fx134 release owner and we can be reached in #release-coordination on Slack if you have further process questions.
https://whattrainisitnow.com/release/?version=134

Comment 17

[Pascal Chevrel:pascalc]

Nicholas, your patch does not apply to the release branch.

Comment 18

[Nicholas Poon [:Nick]]

Hi Pascal, this patch has been fixed for 135 so it only affects 134. When you say it does not apply, do you mean it isn't applying cleanly?

Comment 19

[Pascal Chevrel:pascalc]

Yes, it needs rebasing for the 134 release branch

Comment 20

[Nicholas Poon [:Nick]]

As discussed offline, I will make a new patch with the fix on bookmarks/release and re-request uplift

Comment 21

[Nicholas Poon [:Nick]]

Attachment: Bug 1936359 - Fix history undo snackbar on fragment change on Fx134

Comment 22

[Nicholas Poon [:Nick]]

Hey Pascal,
As we discussed, I put a patch out for the fix that is rebased with bookmarks/release. The patch has been approved but it says that I need relman to approve and land it. I haven't done something like this before. Will you do this for me when working on the new dot release or do I need to request this separately somehow?

Comment 23

[Phabricator Automation]

release Uplift Approval Request

• User impact if declined: Snackbar blocks toolbar indefinitely (until app restart) on history delete
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Delete a history item, navigate away from history fragment. Then, test that snackbar disappears. Repeat and test that the snackbar's undo action works
• Risk associated with taking this patch: Low
• Explanation of risk level: There are automated tests and I've already tested locally
• String changes made/needed: N/A
• Is Android affected?: yes

Comment 24

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/743da9a878d6

Comment 25

[Lorand Janos]

I managed to reproduce the issue on an earlier build (RC 134.0.1).

Verified as fixed on the latest RC build (134.0.2).

After deleting a history item and navigating back, the undo snackbar is dismissed after a few seconds. The "Undo" button works as expected.

Devices used:

• Google Pixel 9 Pro XL (Android 15);
• Lenovo TB X606X (Android 10).

Marking the ticket as verified on 134.