639102 - Thumbnails cut off

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

Thumbnails in Fennec can be cut off sometimes, particularly with pages that we lay out at different sizes.

STR:
1.) Open popup-blocker: http://www.popuptest.com/
2.) Put Fennec in Portrait mode (CTRL-SHIFT-Q)
3.) Open popups!

Results: Thumbnails have gray strips on the right side

I also think we are making a mistake and asking for the thumbnail to cover the width of the window, and not the width of the document. Upcoming patch takes the minimum value of the document width or passed in width, and moves thumbnail drawing to Browser:FirstPaint so that the page has (hopefully?) had more time to layout.

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch v1

Comment 2

[Matt Brubeck (:mbrubeck)]

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

>+++ b/chrome/content/browser.js

>+      if (browser.currentURI.spec != "about:blank") {
>+        aTab.updateThumbnail();
>+      }

No {}.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch v2

Whoops. This patch had some side effects I only see on device.

In some cases, FirstPaint fires before networkEnd and can result in a strange looking thumbnail (i.e. Google page without google logo, or PMO layout in a very narrow column or with no images).

This checks to see if loading is complete before updating the thumbnail, effectively making this fire after networkEnd or Browser:FirstPaint, whichever is longer.

Reasking for review.

Comment 4

[Wesley Johnston (:wesj)]

Attachment: patch v2

Now uploaded as a patch.

Comment 5

[Matt Brubeck (:mbrubeck)]

Comment on attachment 517548 [details] [diff] [review]
patch v2

>+      if (browser.currentURI.spec != "about:blank")
>+        aTab.updateThumbnail();

There's a bug here - if you load a page, then navigate to about:blank in the same tab, the thumbnail still shows the previous page.  But that bug exists in the old code too, so I'll just file it separately.

Comment 6

[Matt Brubeck (:mbrubeck)]

Filed bug 639625 for the issue in comment 5.

Comment 7

[Wesley Johnston (:wesj)]

Pushed: http://hg.mozilla.org/mobile-browser/rev/630dbc835c53

Comment 8

[Anna (Waverley)]

VERIFIED FIXED on:

Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b13pre) Gecko/20110316 Firefox/4.0b13pre Fennec /4.0b6pre 

Device: Motorola Droid 2 (Android 2.2)