Closed Bug 726272 Opened 8 years ago Closed 8 years ago

[Page Thumbnails] don't capture error pages


(Firefox :: General, defect, minor)

Not set



Firefox 14
Tracking Status
firefox13 --- verified


(Reporter: cpeterson, Assigned: ttaubert)



(Whiteboard: [qa!])


(1 file, 1 obsolete file)

One of my New Tab thumbnails (which happens to be is labeled "Problem loading page" (Firefox's "server not found" error page).

The New Tab should not show "Problem loading page" thumbnails. It could substitute the thumbnail's hostname or just omit the thumbnail entirely until a real page title is found.
Summary: [New Tab Page] includes thumbnail for "Problem loading page" error page → [Page Thumbnails] don't capture error pages
OS: Mac OS X → All
Hardware: x86 → All
Duplicate of this bug: 736054
Attached patch patch v1 (WIP) (obsolete) — Splinter Review
Needs a testcase.
Assignee: nobody → ttaubert
Attached patch patch v2Splinter Review
Patch with a test. I accessed gBrowserThumbnails._shouldCapture() directly because it's really hard to tell whether a thumbnails has *not* been captured.
Attachment #606209 - Attachment is obsolete: true
Attachment #606226 - Flags: review?(dietrich)
Can you instead check channel instanceof nsIHttpChannel && channel.requestSucceeded ?
Comment on attachment 606226 [details] [diff] [review]
patch v2

Review of attachment 606226 [details] [diff] [review]:

what gavin said. is a more explicit test that way.
Attachment #606226 - Flags: review?(dietrich) → review-
So (channel.requestSucceeded == true) and (request.status == 0) because that refers to the internal load of "about:neterror". Should we stick with the old approach or is there any other solution?
Comment on attachment 606226 [details] [diff] [review]
patch v2

Review of attachment 606226 [details] [diff] [review]:

old approach is fine for now. i can't think of any other redirect-to-about scenarios, so let's roll with this and update as needed if bugs start flowing in.
Attachment #606226 - Flags: review- → review+
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 14
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Comment on attachment 606226 [details] [diff] [review]
patch v2

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: error pages can overwrite thumbnails in cases of network outage
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): low risk, very small patch with a test
String changes made by this patch: none
Attachment #606226 - Flags: approval-mozilla-aurora?
This makes it impossible to have thumbnails for pages like about:config (manually pinned). Is that acceptable?
We've never taken screenshots of about: pages (this bug is only about error pages). But that doesn't mean we couldn't.
I thought they were missing because of this patch. I was wrong.

The problem happens in Aurora also, which I believe doesn't contain this patch. Please create a followup bug to show thumbnails for about:config and similar pages.
Comment on attachment 606226 [details] [diff] [review]
patch v2

[Triage Comment]
Low risk patch in support of correctness in a new feature. Approved for Aurora 13.
Attachment #606226 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [qa+]
Page Thumbnails don't capture error pages.
Verified fixed on FF 13b3:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20100101 Firefox/13.0
Whiteboard: [qa+] → [qa!]
Depends on: 805338
You need to log in before you can comment on or make changes to this bug.