1039738 - Private tabs list is cut off for portait orientation

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Brian Nicholson (:bnicholson)]

Open two private tabs, then open the tabs tray: only one private tab is visible. It looks like the ListView height isn't filling the TabsTray as expected.

Comment 1

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

This is caused by bug 1021356 part 1, likely due to the change to a ScrollView.

Comment 2

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

Attachment: Allowing scrolling in private tabs list

Comment 3

[Brian Nicholson (:bnicholson)]

Comment on attachment 8457516 [details] [diff] [review]
Allowing scrolling in private tabs list

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

::: mobile/android/base/resources/layout/private_tabs_panel.xml
@@ +5,5 @@
>  
>  <merge xmlns:android="http://schemas.android.com/apk/res/android"
>         xmlns:gecko="http://schemas.android.com/apk/res-auto">
>  
> +    <ScrollView android:layout_width="match_parent"

Why do we need a ScrollView at all? TwoWayView is already scrollable.

@@ +45,5 @@
>  
> +    </ScrollView>
> +
> +    <!-- Note: scrolling in the TabsTray does not work
> +         unless it is laid out after the empty view. -->

Why?

Also, note that even with this bug, I *can* still scroll the tabs tray -- it's just not the right height. Since the regressing bug changed PrivateTabsPanel to extend ScrollView instead of FrameLayout, I would expect the fix here to be to add fillViewport="true" to the ScrollView. Does that not work? Still curious why a ScrollView is needed in the first place, though.

Comment 4

[Brian Nicholson (:bnicholson)]

(In reply to Brian Nicholson (:bnicholson) from comment #3)
> Why do we need a ScrollView at all? TwoWayView is already scrollable.

Just to be clear, I'm talking about the TabsTray (which extends TwoWayView) in this layout file.

Comment 5

[Brian Nicholson (:bnicholson)]

Comment on attachment 8457516 [details] [diff] [review]
Allowing scrolling in private tabs list

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

I was more focused on bug 1021356 than this one -- the ScrollView comment above isn't relevant to your fix here since you're moving the TabsTray outside of it. I'm still curious about your note. Why does scrolling in the TabsTray not work unless it is laid out after the empty view?

Comment 6

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

(In reply to Brian Nicholson (:bnicholson) from comment #5)
> I was more focused on bug 1021356 than this one -- the ScrollView comment
> above isn't relevant to your fix here since you're moving the TabsTray
> outside of it. I'm still curious about your note. Why does scrolling in the
> TabsTray not work unless it is laid out after the empty view?

I'm not sure. I imagine it has something to do with how the .setEmptyView and related code handle which view (i.e. the empty view and not-so-empty view) is displayed and thus how the click events are swallowed by each view.

I'll add, "For an unknown reason..." to the comment.

Comment 7

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

Attachment: Correct private tabs list appearance

Updated patch (and updated description, given comment 3's clarification.

Comment 8

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

https://hg.mozilla.org/integration/fx-team/rev/a794759a5272

Comment 9

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

Comment on attachment 8457663 [details] [diff] [review]
Correct private tabs list appearance

Move r+.

Comment 10

[Carsten Book [:Tomcat]]

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