Closed Bug 1320550 Opened 3 years ago Closed 3 years ago

"Undo close tab" functionality doesn't properly handle being deactivated

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
fennec + ---
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- wontfix
firefox53 --- verified

People

(Reporter: JanH, Assigned: JanH)

Details

Attachments

(2 files)

STR:
1. Make sure you haven't got any "Recently closed" tabs by clearing history, manually deleting the relevant files or whatever.
2. Set browser.sessionstore.max_tabs_undo to 0 and restart Firefox.
3. Play some media with audio, e.g. a Youtube video.
4. Close the tab.

Expected:
1. Tab closes and video playback stops.

Actual:
1. Tab closes in the UI, however video playback is still audible in the background.

The problem seems to be that the code for preparing the snackbar never expects the closedTabData sent by the session store to be empty (https://dxr.mozilla.org/mozilla-central/rev/26773c7afa55b6f8ad725d5a00850b379eb0a10f/mobile/android/chrome/content/browser.js#1251), so the uncaught exception then interrupts the remainder of browser.js's closed tab handling.

There's also a related problem in that the closed tab data is only updated when browser.sessionstore.max_tabs_undo is > 0. If the user sets this to 0, we stop updating the list of recently closed tabs but don't clear it, either. In combination with the tab restore setting set to "Always restore" this means that those old closed tabs can be preserved indefinitely until the user eventually clears the browsing history. This means that if a user closes a tab with max_tabs_undo = 0 but some recently closed tabs still around, the "Undo close tab" snackbar that appears will refer to the last tab the user closed before setting max_tabs_undo to 0.
Comment on attachment 8814712 [details]
Bug 1320550 - Part 1 - Only try showing the "Undo close tab" snackbar if we actually have some closed tab data.

https://reviewboard.mozilla.org/r/95822/#review95938
Attachment #8814712 - Flags: review?(s.kaspari) → review+
Comment on attachment 8814713 [details]
Bug 1320550 - Part 2 - Clear closed tabs when max_tabs_undo is set to 0.

https://reviewboard.mozilla.org/r/95824/#review95940
Attachment #8814713 - Flags: review?(s.kaspari) → review+
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/849f80eca8e6
Part 1 - Only try showing the "Undo close tab" snackbar if we actually have some closed tab data. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/bcc32212520f
Part 2 - Clear closed tabs when max_tabs_undo is set to 0. r=sebastian
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/849f80eca8e6
https://hg.mozilla.org/mozilla-central/rev/bcc32212520f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Seems to affect all versions. Worth uplifting?
tracking-fennec: ? → +
Flags: needinfo?(jh+bugzilla)
It's only affecting people that have been digging around in about:config, but at since the effect is still rather bad, at least Part 1 is certainly worth uplifting, especially given how trivial the fix is. Part 2 doesn't seem that urgent, but having it land sooner could still be nice.

So basically, yes. :-)
Flags: needinfo?(jh+bugzilla)
Okay, in this case it can ride the trains (about:config is not really supported).
Hello, I've verified this using LG G4 (Android 6.0).
You need to log in before you can comment on or make changes to this bug.