Closed
Bug 639102
Opened 14 years ago
Closed 14 years ago
Thumbnails cut off
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
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)
4.24 KB,
patch
|
mbrubeck
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•14 years ago
|
||
Assignee: nobody → wjohnston
Attachment #517074 -
Flags: review?(mbrubeck)
Assignee | ||
Updated•14 years ago
|
tracking-fennec: --- → ?
Comment 2•14 years ago
|
||
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+
Updated•14 years ago
|
Attachment #517074 -
Flags: approval2.0+
Updated•14 years ago
|
Whiteboard: [has patch][has review][has approval][can land]
Assignee | ||
Comment 3•14 years ago
|
||
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)
Assignee | ||
Comment 4•14 years ago
|
||
Now uploaded as a patch.
Attachment #517546 -
Attachment is obsolete: true
Attachment #517546 -
Flags: review?(mbrubeck)
Attachment #517548 -
Flags: review?(mbrubeck)
Comment 5•14 years ago
|
||
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+
Comment 6•14 years ago
|
||
Filed bug 639625 for the issue in comment 5.
Assignee | ||
Comment 7•14 years ago
|
||
Assignee | ||
Updated•14 years ago
|
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 8•14 years ago
|
||
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
Updated•14 years ago
|
tracking-fennec: ? → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•