1109155 - [7" Tablet] Gridview doesn't show the last row completely on portrait

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[u421692]

Attachment: device-2014-12-09-184045.png

Devices:
Samsung Galaxy Tab 2 (Android 4.2.2) ; Nexus 7 (Android 5.0.1)

Steps:
1. Open more tabs open than will fit on a single screen

Actual result:
Gridview doesn't show the last row completely on portrait

Note: It's basically the same issue as in bug 1100317, more padding is needed.

Comment 1

[Lucas Rocha (:lucasr)]

I suspect this is regression from bug 1098433. Taking.

Comment 2

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

This may not be a regression and may just be bug 1104133 - see bug 1091587 for some of my testing/diagnosis attempts.

However, I was using strange layout params so it's possible I just screwed up my testing attempts.

Comment 3

[Martyn Haigh (:mhaigh)]

Attachment: Gridview doesnt show the last row completely on portrait

The GridView seems to have an issue in determining it's own height when we use padding with ClipToPadding set to false.  The top padding seems to push the content down without the total height of the gridview increasing to accommodate, hence we have to make an allowance for that when setting the bottom padding.

Comment 4

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

Comment on attachment 8544035 [details] [diff] [review]
Gridview doesnt show the last row completely on portrait

This might be caused by some bug in GridView that doesn't compensate for the width of the selection highlight. I guess we need to compensate for it manually.

Comment 5

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

Comment on attachment 8544035 [details] [diff] [review]
Gridview doesnt show the last row completely on portrait

Moving to r+

Comment 6

[Martyn Haigh (:mhaigh)]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/07f515f6fb63

Comment 8

[u421692]

Verified as fixed on latest Nightly on Nexus 7 (Android 5.0.1)

Comment 9

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

Request uplift to 36 (beta), please.

Comment 10

[Martyn Haigh (:mhaigh)]

Attachment: Gridview doesn't show the last row completely on portrait

mcomella: my beta build is broken - any chance I can get you to sanity check this for uplift to beta for me?

Comment 11

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

(In reply to Martyn Haigh (:mhaigh) from comment #10)
> mcomella: my beta build is broken

Also broken.

Comment 12

[Martyn Haigh (:mhaigh)]

Attachment: [Tablet] Gridview doesn't show the last row completely on portrait

Approval Request Comment
[Feature/regressing bug #]:
Last row of the tabs in the new tablet code isn't shown completely

[User impact if declined]:
Users will not be able to see the bottom of the last line of tabs in the new tablet grid view when there are enough tabs to fill the screen. 

[Describe test coverage new/current, TreeHerder]:
Has been working on 37 for a few weeks.

[Risks and why]: 
Minimal risk - we only change the padding in this patch so the only possibility is that the padding is still incorrect.

[String/UUID change made/needed]:
None

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bc4e060096bd

Comment 14

[Teodora Vermesan (:TeoVermesan)]

Attachment: last row behavior.png

Tested with:
Device: Nexus 7 (Android 5.0.1)
Build: Firefox for Android 36 Beta 6
I opened 3 x 4 tabs to fit the screen.
On 36 Beta 6, in portrait mode, to reveal the last row completely you have to scroll. A space is displayed between tabs and the bottom of the screen
On 38 latest Nightly and Aurora, in portrait mode, you do not have to scroll to reveal the last row.
Shouldn't the behavior be consistent?

Comment 15

[Flaviu Cos]

Verified as fixed in Firefox 37 Beta 1;
Device: Google Nexus 7 (Android 5.0.2).