Closed Bug 639102 Opened 9 years ago Closed 9 years ago

Thumbnails cut off

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: wesj, Assigned: wesj)

Details

(Keywords: polish, Whiteboard: [has patch][has review][has approval][can land])

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Patch v1 (obsolete) — Splinter Review
Assignee: nobody → wjohnston
Attachment #517074 - Flags: review?(mbrubeck)
tracking-fennec: --- → ?
Keywords: polish
Comment on attachment 517074 [details] [diff] [review]
Patch v1

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

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

No {}.
Attachment #517074 - Flags: review?(mbrubeck) → review+
Attachment #517074 - Flags: approval2.0+
Whiteboard: [has patch][has review][has approval][can land]
Attached file Patch v2 (obsolete) —
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.
Attachment #517074 - Attachment is obsolete: true
Attachment #517546 - Flags: review?(mbrubeck)
Attached patch patch v2Splinter Review
Now uploaded as a patch.
Attachment #517546 - Attachment is obsolete: true
Attachment #517546 - Flags: review?(mbrubeck)
Attachment #517548 - Flags: review?(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.
Attachment #517548 - Flags: review?(mbrubeck) → review+
Filed bug 639625 for the issue in comment 5.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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)
Status: RESOLVED → VERIFIED
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.