1139202 - Create tabs panel thumbnail for about:privatebrowsing pages

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: Screenshot_2015-03-03-15-12-36.png

Tested on my N9, Nightly, in Private Browsing mode.

Comment 1

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

Mike or Martyn - Can one of you pick this up and investigate. Not a high priority, but worth getting it fixed.

Comment 2

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

Martyn, have you encountered any of this code when working on the tabs tray, and do you have a device with which you can repro? If not, I can take a look.

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

We think this might be N9 only given that its aspect ratio is different than other devices.

Comment 4

[Martyn Haigh (:mhaigh)]

Mike: Yeah - encountered this code whilst doing the tablet refresh.  I've got a N9 so will investigate.  Personally I think we need to completely rework how these thumbnails are generated as currently it's ugly as sin.

Comment 5

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

NI self to look at today to meet with mhaigh later.

Comment 6

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

Attachment: Screenshot of various content

After discussing with mhaigh and antlam, it turns out there are three categories of pages:
  * Web content (HTML)
  * about: pages (XUL)
  * Android pages

Web content works great (only pages with no scrolling are problematic), about: pages are inconsistent (about:config is fine, about:downloads falls off the right side of the page), and Android content is the most noticeably broken.

Things worth noting:
  * This gets worse the longer the width is than the height but in these cases, users are less likely to use the device in landscape (I assume)
  * Most users don't open about:privatebrowsing and then immediately open the tabs panel to see the broken thumbnail

Comment 7

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

Consensus: we'll probably leave xul alone for now because it works well enough, but we should fix the android content pages. The only one that is really an issue that I know of is about:privatebrowsing.

We think the issue is because on web content, the content can generally scroll down, allowing the thumbnail to use vertical content not currently on the screen to maintain a reasonable aspect ratio, while the private tabs panel (and xul pages without scroll, e.g. about:downloads) has a fixed height which is the screen height.

One solution would be to replace the about:privatebrowsing thumbnail with a private browsing specific asset, similar to about:home (i.e. the globe in a grey rectangle). We could consider similar approaches for other about: pages. Two choices we ruled out were 1) still making the horizontal crop, but centering the content and 2) letterboxing the thumbnail.

NI Anthony for designs.

Comment 8

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: tabprev_masq.zip

I'd like to see us land bug 1018456 so I think this is really an interim solution. But here's the masq that we could put in place of the globe.

:)

Comment 9

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Not a blocker on V1. 

Also, after some experimenting and more thinking. The blur effect should be an experiment right now.

Comment 10

[Martyn Haigh (:mhaigh)]

Attachment: MozReview Request: Bug 1139202 - Create tabs panel thumbnail for about:privatebrowsing pages; r?mcomella

Bug 1139202 - Create tabs panel thumbnail for about:privatebrowsing pages; r?mcomella

Comment 11

[Martyn Haigh (:mhaigh)]

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

Comment 12

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

Comment on attachment 8649946 [details]
MozReview Request: Bug 1139202 - Create tabs panel thumbnail for about:privatebrowsing pages; r?mcomella

https://reviewboard.mozilla.org/r/16515/#review14961

lgtm after extending ThemedImageView, removing the pb state code, and using a universal getDrawable method.

::: mobile/android/base/resources/drawable/tab_panel_tab_background.xml:21
(Diff revision 1)
> +    <item>

nit: add newline above.

::: mobile/android/base/widget/ThumbnailView.java:77
(Diff revision 1)
> +    public int[] onCreateDrawableState(int extraSpace) {

Private state control is already given in ThemedImageView – extend that and remove the private state code within this class.

::: mobile/android/base/widget/ThumbnailView.java:96
(Diff revision 1)
> -            drawable = getResources().getDrawable(R.drawable.tab_panel_tab_background);
> +            drawable = getResources().getDrawable(R.drawable.tab_panel_tab_background, null);

This is an API 21 method: http://developer.android.com/reference/android/content/res/Resources.html#getDrawable%28int,%20android.content.res.Resources.Theme%29

The best alternative, from my experience, is to use ContextCompat.getDrawable instead: http://developer.android.com/reference/android/support/v4/content/ContextCompat.html#getDrawable%28android.content.Context,%20int%29

Comment 13

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

Also, don't forget to compress these new images (if rb didn't lie to me).

Comment 14

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=49c7d8217349

Comment 15

[Martyn Haigh (:mhaigh)]

url:        https://hg.mozilla.org/integration/fx-team/rev/2cc38c418be0e4f1a809da93c3d2fd2de468d4b2
changeset:  2cc38c418be0e4f1a809da93c3d2fd2de468d4b2
user:       Martyn Haigh <[e-mail]>
date:       Fri Aug 21 11:16:46 2015 +0100
description:
Bug 1139202 - Create tabs panel thumbnail for about:privatebrowsing pages; r=mcomella

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2cc38c418be0

Comment 17

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot_2015-10-23-14-24-26.png

Verified as fixed using:
Device: Nexus 7 (Android 5.1)
Build: Firefox for Android 44.0a1 and 43.0a1 (2015-10-21)