825176 - [Tablet] The forward button is hidden when closing the Tab Drawer

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Adrian Tamas (:AdrianT)]

Nightly 20.0a1 2012-12-27
Asus EEE Transformer TF101 (Android 4.0.3)

Steps to reproduce:
1) Open 3 different pages in the same tab a a new one in a second tab
2) In the first tab go back to the second page so you have both the back and forward buttons displayed
3) Close the Tabs Drawer

Expected results:
The Tab Drawer is closed and the URL bar controls are not affected

Actual results:
The Forward button is hidden and a white background "leaks" in the Tab Count indicator area. The forward button does not reappear unless the user moves to a different tab and back and the Tab Drawer is opened.

Note: 
This is probably caused buy the new Tab Drawer UI

Comment 1

[Sriram Ramasubramanian [:sriram]]

Wes was working on hiding the forward button.

Comment 2

[Wesley Johnston (:wesj)]

I think something is missing from the steps to reproduce? The tabs drawer isn't open when I go to step 3.

I can open two tabs, one with forward hidden and one with it showing. When I switch between them everything updates correctly for me, so I must be missing something.

Comment 3

[Wesley Johnston (:wesj)]

Oh wait. I'm on a Nexus 7. (Charing my 10" tablet). I'll try there in a bit.

Comment 4

[Wesley Johnston (:wesj)]

Ahh. I can reproduce this now.

Comment 5

[Wesley Johnston (:wesj)]

Attachment: Patch v1

The tabs animation adjusts the translation of the forward button. So does the hiding animation. We can either make them aware of each other (which is a pretty simple patch that will fall apart next time we do this I assume), or adjust the real margin of the forward button in its transition, which is what I'm doing here.

Comment 6

[Wesley Johnston (:wesj)]

Comment on attachment 696440 [details] [diff] [review]
Patch v1

Since lucas is on vacation, moving review.

Comment 7

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 696440 [details] [diff] [review]
Patch v1

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

This looks good to me.

::: mobile/android/base/BrowserToolbar.java
@@ +1041,5 @@
>                  }
> +
> +                ViewGroup.MarginLayoutParams layoutParams =
> +                    (ViewGroup.MarginLayoutParams)mForward.getLayoutParams();
> +                layoutParams.leftMargin = mDefaultForwardMargin + (mForward.isEnabled() ? mForward.getWidth()/2 : 0);

"width" can be used here as it is final.

@@ +1059,3 @@
>              anim.attach(mForward,
>                        PropertyAnimator.Property.TRANSLATION_X,
> +                      -1*width);

"-width" would do.

Comment 8

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e0ffa0535cc

Comment 9

[Josh Aas]

Backed out for Android startup crashes on advice of others.

http://hg.mozilla.org/integration/mozilla-inbound/rev/f0481ac71e08

Comment 10

[Wesley Johnston (:wesj)]

Whoops. Moved the dimens to the global layout/dimens.xml file instead. It will be ignored on other form factors.

https://hg.mozilla.org/integration/mozilla-inbound/rev/83b7d05f5928

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/83b7d05f5928

Comment 12

[Adrian Tamas (:AdrianT)]

Issue is no longer reproducible on Aurora 20.0a2 2013-01-10 or Nightly 21.0a1 2013-01-10 on the Asus EEE Transformers TF101 (Android 4.0.3). Marking the issue as verified