89151 - Dynamically generated GIF images won't print

Status: VERIFIED FIXED

(Core :: Printing: Output, defect, P1)

Description

[Cavin Song]

BuildID: 2001070303

The map does not show up at all on paper. 4.x prints out fine.

Comment 1

[sujay]

Yup, its not printing out....

I used MapQuest to print out driving directions with an image
and that works fine.

cc: tpreston

Comment 2

[Eric Hokanson]

It appears any large image won't print completely or not at all.  It looks like
the browser is trying to redownload the image and prints before it's finished.

Comment 3

[sujay]

we have several bugs like this. see 84075, 92849 , 94014, 94169..
all the same issue. image/map does not get printed out.

Comment 4

[sujay]

*** Bug 84075 has been marked as a duplicate of this bug. ***

Comment 5

[sujay]

*** Bug 92849 has been marked as a duplicate of this bug. ***

Comment 6

[sujay]

*** Bug 94014 has been marked as a duplicate of this bug. ***

Comment 7

[sujay]

*** Bug 94169 has been marked as a duplicate of this bug. ***

Comment 8

[sujay]

target milestone = 0.9.4

Don, this is the master bug, you can look at the DUPs for clues.

Comment 9

[Ashley Bischoff (blog at handcoding.com)]

*** Bug 95745 has been marked as a duplicate of this bug. ***

Comment 10

[Ashley Bischoff (blog at handcoding.com)]

Similar to bug 95745 ("Mozilla doesn't seem to print images that are dynamically
generated"), bug 94014 mentions:

"there seem to be problems with printing large images or 
images generated on the fly.  I bet this falls into that catergory.  I bet the 
image is not in memory yet and printing is notified it is."-- [e-mail]

Perhaps that's the origin of this problem?

Comment 11

[dcone (gone)]

Yes.. that is exactly what the problem is.

Comment 12

[sujay]

*** Bug 97494 has been marked as a duplicate of this bug. ***

Comment 13

[sujay]

*** Bug 97842 has been marked as a duplicate of this bug. ***

Comment 14

[lohphat]

I searched on this with "alt tag print" before submitting my dup report. 
Perhaps revising the summary to indicate alt tag is printed instead *might*
reduce further dupes.  But then again, it might not.  So there.

Comment 15

[Christian Reis]

*** Bug 90267 has been marked as a duplicate of this bug. ***

Comment 16

[Christopher Aillon (sabbatical, not receiving bugmail)]

*** Bug 98499 has been marked as a duplicate of this bug. ***

Comment 17

[Markus Gerstel]

Possible duplicates : Bug 95495 and Bug 98499

Comment 18

[Ashley Bischoff (blog at handcoding.com)]

*** Bug 90822 has been marked as a duplicate of this bug. ***

Comment 19

[dcone (gone)]

*** Bug 99558 has been marked as a duplicate of this bug. ***

Comment 20

[Christopher Aillon (sabbatical, not receiving bugmail)]

*** Bug 99731 has been marked as a duplicate of this bug. ***

Comment 21

[Christopher Aillon (sabbatical, not receiving bugmail)]

*** Bug 93686 has been marked as a duplicate of this bug. ***

Comment 22

[Christopher Aillon (sabbatical, not receiving bugmail)]

*** Bug 100718 has been marked as a duplicate of this bug. ***

Comment 23

[Jussi-Pekka Mantere]

Renaming bug; adding keywords.

Another test case: http://www.best.com/~jpm/bugzilla/89151/ .

Comment 24

[Jussi-Pekka Mantere]

BTW, Pragma: no-cache in the HTTP headers doesn't make a difference:
http://www.best.com/~jpm/bugzilla/89151/nonocache.html .

Comment 25

[lchiang]

this may be an item for topembed.  cc: chofmann

Comment 26

[Kevin McCluskey (gone)]

*** Bug 88895 has been marked as a duplicate of this bug. ***

Comment 27

[Kevin McCluskey (gone)]

cc'ing Vidur. He is working on a solution.

Comment 28

[vidur (gone)]

Attachment: Proposed fix for dynamic image printing problem

Comment 29

[vidur (gone)]

The proposed fix does the following:
1) Allows pres contexts to override the load flags sent to the image loader. The
printing pres context uses nsIRequest::LOAD_FROM_CACHE to make sure that the
image comes out of the cache.
2) Gets the image loader to actually check for caching and validation flags.
Also adds a cache key parameter to LoadImage. This will be used (not yet in this
fix) for images that are results of a HTTP posts.
3) Shifts the responsibility of dooming expired decoded image cache entries from
the generic cache service to the image cache. If nsIRequest::LOAD_FROM_CACHE is
passed in (as it now will be for printing), expiration times are ignored. In the
onscreen case, expiration times are checked in the same way as they were in the
http protocol handler.4) Removed creation of one of the presshell/frame models
from nsDocumentViewer.cpp. It served no purpose.

There's a little code from the fix from bug 84017 in the patch for
nsDocumentViewer.cpp - apologies.

Comment 30

[Darin Fisher]

are there any cases other than printing in which imglib is asked to load an
image with LOAD_FROM_CACHE set?  if so, then we might have a problem with the
SetCacheKey call... in particular, passing PR_TRUE as the second parameter means
that the image will _never_ be fetched from the network.

there are some problems which may result even for printing:
 - the image could be too large to fit in the cache
 - the cache size could be too small to fit the image(s)

what happens in the failure case?

[talked a bit with vidur over the phone, and i think he addressed these
issues... i'm just posting this for reference sake]

Comment 31

[vidur (gone)]

Printing is the only case currently where LOAD_FROM_CACHE will be passed in. To
be frank, the code to pass in the cache key and set it on the channel will
probably not be exercised for printing...or if it is, it's not going to do the
right thing since it will result in an asynchronous load of the image. The cache
key will be useful in the next iteration of the patch when I deal with images
that are returned as a result of a HTTP post (currently the image cache
incorrectly overwrites such images in the cache).

The hope is that, for printing, we'll find the image in the decoded image cache.
The patch makes sure that we don't doom cache entries that would otherwise have
been tagged as expired. Darin, you're right - I don't currently deal with
failure cases because of cache size. I need to confirm that in the cases you
mention, image requests don't make it into the decoded image cache.

Comment 32

[vidur (gone)]

After talking it over with Rick Potts, we decided that a slightly modified
version of this patch would be safe to get on the branch. The following bugs
will still exist:
1) Images that are not kept in the decoded image cache because they are too
large or have already been evicted for size reasons will not print correctly.
2) Images that were returned as a result of a HTTP post will not print correctly.

The patch posted below gets rid of the changes to the load flags used for the
HTTP channel - if we don't find the image in the decoded image cache, loading
and decoding it subsequently won't work since printing is currently synchronous.
I've left the change that adds a cache key parameter to imgILoader::LoadImage
since this will be necessary for dealing with HTTP posts. Since the cache key
isn't used yet, the change is safe.

Comment 33

[vidur (gone)]

Attachment: Slightly simpler and safer fix for the branch.

Comment 34

[rpotts (gone)]

Comment on attachment 51297 [details] [diff] [review]
Slightly simpler and safer fix for the branch.

sr=[e-mail]

Comment 35

[Darin Fisher]

Comment on attachment 51297 [details] [diff] [review]
Slightly simpler and safer fix for the branch.

r=darin on everything except the nsDocumentViewer.cpp 
stuff (i'm not at all familiar with that code).

Comment 36

[Jaime Rodriguez, Jr.]

pls check it in today - tantative PDT+, pending a r=.

let's have qa run browser buster, printing test and exercise the cache.

Comment 37

[vidur (gone)]

Got a verbal r= for the nsDocumentViewer.cpp changes from [e-mail].

Fix checked into the MOZILLA_0_9_4_BRANCH on 9/28 at 8:21pm.

Comment 38

[Ashley Bischoff (blog at handcoding.com)]

*** Bug 93272 has been marked as a duplicate of this bug. ***

Comment 39

[Ashley Bischoff (blog at handcoding.com)]

*** Bug 95495 has been marked as a duplicate of this bug. ***

Comment 40

[vidur (gone)]

Fix checked into the trunk on 9/29.

Comment 41

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Heh, not just verbal: I added the has-review flag to the patch ;) Maybe your
Bugzilla mail options need some adjusting if you were expecting that mail...?

Comment 42

[sujay]

okay, just verified this on Windows 10/1 branch build...looks good!
I went through most of the DUPS on this bug and made sure they
all printed the dynamicically generated image/page.

Vidur, do you wanna mark this RESOLVED-FIXED ?

I will test on trunk later.

Comment 43

[sujay]

also verified on Mac 10/1 branch build.

Comment 44

[Jaime Rodriguez, Jr.]

Marking fix, this was checked into the trunk and branch.

Comment 45

[sujay]

verified in 10/1 branch build for Mac and windows.

will verify on trunk later.

Comment 46

[Tom Everingham]

verified that images are pulled from cache for printing - 10-01 Windows NT4

Comment 47

[vidur (gone)]

I'm going to post an alternate patch that grabs an image for printing from the
onscreen presentation. Getting it from the onscreen frame model is a safer bet
than finding it in the decoded image cache. The patch fails over to the existing
codepath if it can't find an onscreen version of the image. Since we now store
images at their natural dimensions and depth (and scale on-the-fly), there
should be no problem with reusing the onscreen image request.

Comment 48

[vidur (gone)]

Attachment: new patch that uses the onscreen image

Comment 49

[lohphat]

What if the decoded image is of high quality but the screen is of lower quality
than the printer.  Will the poorer screen image be sent to the printer or will
the imgae be decoded again to match printer resolution?

Comment 50

[vidur (gone)]

The image stored in memory is stored at the depth of the original encoded image
(1-8 bit for GIF, 16-bit for JPEG, etc.) and is scaled up or down on-the-fly to
the device it's being rendered to.

Comment 51

[Ken]

What's the ETA for that patch to the 0.9.4 branch?

Comment 52

[sujay]

verified in 10/29 trunk also.

Comment 53

[Andrew Hagen]

*** Bug 95495 has been marked as a duplicate of this bug. ***

Comment 54

[Michael Dunn]

Apparently this fix is not on the branch can we push it?
See: http://bugscape.netscape.com/show_bug.cgi?id=11675#c2
I am reopening until fix is on the branch and verified.
I am plusing since it has been verified on the trunk.

Comment 55

[sujay]

But this bug was fixed on the branch also...

Here is Vidur's comments:

"Fix checked into the MOZILLA_0_9_4_BRANCH on 9/28 at 8:21pm."

Comment 56

[sujay]

re-verified on 1/7 branch and trunk.

this is fixed..

REOPENED the other bug:
http://bugscape.netscape.com/show_bug.cgi?id=11675#c2

need more info in that other bug.

Comment 57

[sujay]

verified on 1/7 branch and trunk.

Comment 58

[Michael Dunn]

OK. This fix is there. Removing KW