1204760 - Undo close tab toast covers tab bar

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

When I want to close several tabs I just opened, I used to swipe away the bottom tab repeatedly. Note after I swipe away a tab the toast covers the tabs tray preventing me from closing other recent tabs.

Comment 1

[:Margaret Leibovic]

I agree this has bothered me. This issue came up before in bug 1019735, but it was a bit different, since the toast was blocking access to the urlbar.

mhaigh, can you link this to whatever meta bug you're using to track these tabs tray issues?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

I bet moving to Snackbars instead of Toasts is the way to fix this.

Comment 3

[Martyn Haigh (:mhaigh)]

mfinkle: agreed - the snackbar is dismissable which will be a huge win in situations like this.

Comment 4

[:Margaret Leibovic]

Maybe time to re-prioritize bug 1157526...

Comment 5

[:Margaret Leibovic]

Fullscreen tabs tray is only on Nightly.

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

It looks like a snackbar is our only proposed solution for this? The snackbar and toast APIs are very similar. We could use this here as a first step into replacing toasts. Snackbar will require the Android design support library (bug 1189306) but now with simple AAR support this should be easy to add.

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1204760-undo-snackbar.patch

Patch to show Snackbar instead of Toast (Depends on patch in bug 1215026)

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

If you want to run this locally then you will need to import the patches from the following bugs:
* bug 1189306
* bug 1215026

I didn't land them yet because this here is the first feature that will need those changes.

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8676805 [details] [diff] [review]
1204760-undo-snackbar.patch

Review of attachment 8676805 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! Is there a Java entry point to close tab that we may need to cover too?

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=87db0ab3a9b5

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Michael Comella (:mcomella) from comment #9)
> Looks good! Is there a Java entry point to close tab that we may need to
> cover too?

From what I can see this is the only place. This code path is triggered by the "Tab:Close" event.

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

NI: I wonder if I should push this with its dependencies (Android design support library: bug 1189306, Snackbar API: bug 1215026) today or if I should hold it back until the Fx45 cycle. There's an issue on Android 2.3/4.0 (bug 1217416) - but this has been fixed and is waiting for review.

Comment 13

[:Margaret Leibovic]

(In reply to Sebastian Kaspari (:sebastian) from comment #12)
> NI: I wonder if I should push this with its dependencies (Android design
> support library: bug 1189306, Snackbar API: bug 1215026) today or if I
> should hold it back until the Fx45 cycle. There's an issue on Android
> 2.3/4.0 (bug 1217416) - but this has been fixed and is waiting for review.

I think we could just push it now. We still have a week until the merge, and it would be nice to use that time to start getting more test coverage for these changes.

Worst case, if we end up with too many regressions to tackle in 44, we can back out these changes (right?).

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to :Margaret Leibovic from comment #13)
> I think we could just push it now. We still have a week until the merge, and
> it would be nice to use that time to start getting more test coverage for
> these changes.

Alright. :)

> Worst case, if we end up with too many regressions to tackle in 44, we can
> back out these changes (right?).

Yeah, we would need to back out 3-4 patches (If we want to completely rollback everything including Android Design Support Library). A bit annoying maybe but doable.

Comment 15

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/aadaca56ce9962a7675e1b0dd88a52263c097ce9
Bug 1204760 - Use snackbar instead of toast for "undo close tab" notification. r=mcomella

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

I approve this landing

Comment 17

[Carsten Book [:Tomcat]]

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