Fenix: [Bug] (regression) "tab closed" snackbar no longer disappears

Created on 18 Jan 2020  ·  17Comments  ·  Source: mozilla-mobile/fenix

Steps to reproduce

  1. close a tab
  2. open another open tab while the "tab closed" snackbar is still visible

Expected behavior

The snackbar disappears.

Actual behavior

The snackbar stays visible. Pressing the "undo" link closes the snackbar (but does not execute the "undo" action).

This is a recent regression.

Device information

  • Android device: OnePlus 7T Pro McLaren Edition / Android 10
  • Fenix version: master branch revision ffb2e72b97b1d066c040087dca2d2035f7e7e9b0
Snackbar Tabs S2 engverified 🐞 bug

Most helpful comment

Quick workaround: swipe to delete removes the notification!

Simply pressing the "undo" button also removes the snackbar (it does not execute the undo action). ;-)

All 17 comments

@mcomella I bisected the issue. This is a regression from the second commit of #7423 (remove unnecessary CoordinatorLayout in fragment_home).

I'm also seeing this.

@mcomella could we make the root of activity_home.xml a coordinator layout instead of a linear layout? This should probably solve this problem without introducing more nested layouts?

@ekager Thanks for looping me in. I'm not sure that will work because the fragment container relies on the LinearLayout in order to dynamically size it when the Toolbar disappears and CoordinatorLayout is roughly a FrameLayout, according to the docs.

Is the CoordinatorLayout necessary to make snackbars go away? My understanding is that they should be independent over any particular view type. Perhaps it's getting attached to the wrong view?

I tried making the anchorView the root container of the activity, rather than bottom_bar, but now the Snackbar doesn't appear at all. I wonder if this would work correctly if we attached it to the root container of the home fragment.

Also, we may reintroduce the CoordinatorLayout in #7700 so it's possible this will be fixed with that patch.

In the old implementation, the Snackbar would be dismissed when switching between fragments. In the current implementation, the Snackbar remains. I'm not sure why this would be the case.

It's possible the previous findSuitableParent algorithm would choose the CoordinatorLayout at the top of the fragment view hierarchy but, now that there's no CoordinatorLayout, it's selecting the view at the top of the HomeActivity hierarchy, so it's not dismissed when changing fragments. I'm not sure why this would prevent the Snackbar from being hidden altogether.

https://github.com/mozilla-mobile/fenix/blob/e1722b95b3cd3afb5c3d28cd848c505122f782d3/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt#L129

I tried making the anchorView...

I noticed that anchorView was for which view to place the Snackbar above, not the view to which the Snackbar is bound. There is a separate view it's bound to (the first one passed in).

I confirmed the problem is fixed when I make the "suitable parent" of the SnackBar as R.id.homeLayout in FenixSnackbar.kt:

            do {
                if (view is ViewGroup && view.id == R.id.homeLayout) {
                    return view
                }

                if (view is CoordinatorLayout) {
                    // ...

I'm not sure what the "correct" solution is though.

Handing this off to @emalysz, who worked on the original implementation with me.

Quick workaround: swipe to delete removes the notification!

Screenshot_20200126-161308_Firefox_Nightly

Quick workaround: swipe to delete removes the notification!

Simply pressing the "undo" button also removes the snackbar (it does not execute the undo action). ;-)

Reproducing on Beta 3.2.0

Same issue here.

How come stable Fenix (3.2.0) was released with such a visible issue standing? Right now this affects all current versions of FF Preview (release, beta, nightly). Even the migration APK for Fennec has the bug 😅

Waiting for https://github.com/mozilla-mobile/fenix/pull/8085 to land, which fixes this issue.

Verified as fixed on the latest version of Firefox Preview Nightly 2/6 #20370604.

Devices:

  • Nokia 6 (Android 7.1.1);
  • Samsung Galaxy S8 (Android 9);
  • Google Pixel 3 (Android Q).
Was this page helpful?
0 / 5 - 0 ratings

Related issues

thelazyoxymoron picture thelazyoxymoron  ·  3Comments

softvision-miralobontiu picture softvision-miralobontiu  ·  3Comments

andreicristianpetcu picture andreicristianpetcu  ·  3Comments

andreicristianpetcu picture andreicristianpetcu  ·  3Comments

phileastv picture phileastv  ·  3Comments