1221030 - Wrong tab thumbnail shape

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot from 2015-11-03 11:10:31.png

Steps to reproduce:
1. Open Firefox
2. Open a new tab
3. Open Tab Tray

Actual results:
- On Firefox for Android 43 Beta 1, thumbnails are displayed in tab tray in a  rectangular shape

Expected results:
- On Firefox for Android 42, Firefox for Android 44.0a2 and Firefox for Android 45.0a1, thumbnails are displayed in tab tray in a square shape.

Please see the attached screenshot

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Seems like some change made it to 43 even though we had some Nightly flags in place.

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

When we introduced the new aspect ratios we accidentally removed the tablet special case:
* https://hg.mozilla.org/mozilla-central/rev/62a3a7f33f09
* https://hg.mozilla.org/mozilla-central/rev/5afeefb4e7f1

In beta we always return the aspect ratio used for top sites:
http://hg.mozilla.org/releases/mozilla-beta/file/fcf963bf85da/mobile/android/base/tabs/TabsPanelThumbnailView.java

I'll create a patch as soon as I'm able to clone mozilla-beta here.. :)

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=611cd566d2c7

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Sebastian Kaspari (:sebastian) from comment #3)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=611cd566d2c7

It turns out to be quite hard to build beta locally (Different mozconfig syntax, different third-party library requirements, I can't downgrade my play-services, ..). So I just pushed a patch to try.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 43 since this is a recent regression.

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Sebastian Kaspari (:sebastian) from comment #4)
> (In reply to Sebastian Kaspari (:sebastian) from comment #3)
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=611cd566d2c7
> 
> It turns out to be quite hard to build beta locally (Different mozconfig
> syntax, different third-party library requirements, I can't downgrade my
> play-services, ..). So I just pushed a patch to try.

Unfortunately there are no APKs of this build:
http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-03fb9e228aedb9ae4e2a0150b17b53740d0cde16/try-android-api-11/

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1472d15e7856

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1221030 - Tablets: Restore tab thumbnail aspect ratio. r?mcomella.

Bug 1221030 - Tablets: Restore tab thumbnail aspect ratio. r?mcomella.

Comment 9

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

Comment on attachment 8683700 [details]
MozReview Request: Bug 1221030 - Tablets: Restore tab thumbnail aspect ratio. r?mcomella.

https://reviewboard.mozilla.org/r/24413/#review21979

Nice digging.

::: mobile/android/base/tabs/TabsPanelThumbnailView.java:40
(Diff revision 1)
>              return ThumbnailHelper.TABS_PANEL_THUMBNAIL_ASPECT_RATIO;

This value is slightly different than what we had before (it's [0.83](https://hg.mozilla.org/mozilla-central/rev/62a3a7f33f09#l1.13) rather than the [0.9](https://hg.mozilla.org/mozilla-central/rev/62a3a7f33f09#l7.13) we had before), but that is only concerning if we care to ship the (minor) tablet changes at the same time as the more visible phone changes.

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Michael Comella (:mcomella) from comment #9)
> This value is slightly different than what we had before (it's
> [0.83](https://hg.mozilla.org/mozilla-central/rev/62a3a7f33f09#l1.13) rather
> than the
> [0.9](https://hg.mozilla.org/mozilla-central/rev/62a3a7f33f09#l7.13) we had
> before), but that is only concerning if we care to ship the (minor) tablet
> changes at the same time as the more visible phone changes.

That's true. I chose to use the new value because the changes to TabGridLayout are already in beta and that's the value we have been testing this with in Nightly.

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8683700 [details]
MozReview Request: Bug 1221030 - Tablets: Restore tab thumbnail aspect ratio. r?mcomella.

Approval Request Comment

[Feature/regressing bug #]: This is a regression that has been hidden because we have been using a new layout in Nightly (flag). This is only in Beta (Firefox 43).

[User impact if declined]: Wrong aspect ratio of thumbnails in tabs tray (See screenshot).

[Describe test coverage new/current, TreeHerder]: Local testing with build from try.

[Risks and why]: Low. This patch just uses a different aspect ratio for tablets. In the worst case this different aspect ratio introduces other regressions. But as this is the aspect ratio that we are using in Aurora and Nightly I don't expect problems.

[String/UUID change made/needed]: -

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8683700 [details]
MozReview Request: Bug 1221030 - Tablets: Restore tab thumbnail aspect ratio. r?mcomella.

OK to uplift, minor fix to UI for tablets.

Comment 13

[Sebastian Kaspari (:sebastian; :pocmo)]

Closing (There's nothing to do on Nightly/Mozilla-Central). Uplift will be tracked by flags.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/07a3f09fbd1c

Comment 15

[u549602]

Verified as fixed on latest Beta(43.0b6)on tablet devices. This was tested on Sony Xperia Z2 (Android 5.0.2).