704185 - PDF content is not properly saved

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Cristian Nicolae (:xti)]

Attachment: PDF sample - actual result

Mozilla/5.0 (Android;Linux armv7l;rv:11.0a1)Gecko/20111121
Firefox/11.0a1 Fennec/11.0a1
Devices: Samsung Galaxy Nexus S
OS: Android 2.3.4

Steps to reproduce:
1. Open Fennec App
2. Browse to www.aljazeera.com
3. Tap on device Menu button > Save as PDF
4. Check if the PDF content is properly saved

Expected result:
The PDF content is saved correctly. All pictures, texts, layouts etc, are displayed the same as they are for the original webpage opened at step 2.

Actual result:
Pictures are not displayed in the PDF file as you can see in the attached file.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

We currently "print" the webpage as a PDF. Does the webpage print OK on desktop?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Also, if you're going to use a summary like "PDF content is not properly saved", I assume you mean PDF files are _never_ saved correctly. Is that true? or is this the only website you have found so far?

Comment 3

[Cristian Nicolae (:xti)]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Also, if you're going to use a summary like "PDF content is not properly
> saved", I assume you mean PDF files are _never_ saved correctly. Is that
> true? or is this the only website you have found so far?

All PDF files saved so far, were not saved properly (webpages like: news.google.com - Classic version, planet.mozilla.org, www.yahoo.com - Desktop version etc). The PDF image content is missing.

Comment 4

[Cristian Nicolae (:xti)]

(In reply to Mark Finkle (:mfinkle) from comment #1)
> We currently "print" the webpage as a PDF. Does the webpage print OK on
> desktop?

It seems that everything works fine on Desktop when a webpage is printed.

Comment 5

[Doug Turner (:dougt)]

Redacting content. It is a feature!

Comment 6

[:Margaret Leibovic]

I was able to reproduce this, and it looks like we're just having problems with the images. I tried some other sites, and it seems to only happen to some images. Is there some layout person we can cc here who might know about this stuff?

Comment 7

[Doug Turner (:dougt)]

the theory is that images are in their own layer, and a

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I doubt layers are involved.

Comment 9

[Joe Drew (not getting mail)]

Doug's comment is too short

Much more likely is that we're not redecoding the images. In Fennec, it's very likely that anything offscreen has been discarded (even things that are currently on-screen can be discarded!). This isn't the case on desktop, in which the frontmost tab always has all its images locked into memory, not discardable.

Whenever we print, we need to explicitly redecode images synchronously. Unfortunately I have no idea how the printing codepath differs from the "regular" codepath. Olli might, though.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

On DOM side the codepath is almost the same, since the original document is
cloned to a new document, images are also cloned, and the new document is printed.

But I don't know if we force re-decode. clone-document-for-printing was, IIRC, implemented before
lazy re-decode was enabled. Do we have some way to force images to be always decoded?

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

nsSimplePageSequenceFrame::PrintNextPage does
      nsLayoutUtils::PaintFrame(renderingContext, mCurrentPageFrame,
                                drawingRegion, NS_RGBA(0,0,0,0),
                                nsLayoutUtils::PAINT_SYNC_DECODE_IMAGES);
so images should be decoded synchronously while printing.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #11)
> so images should be decoded synchronously while printing.

So does this disprove Joe's theory that we're not redecoding the images? If so, do we have any alternative theories?

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It suggests Joe's guess was incorrect.

Comment 14

[JP Rosevear [:jpr]]

The implementation of save to pdf is here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#472

It appears that we use the download manager to fetch the page and then print it.  Is that expected to work?

Comment 15

[Wesley Johnston (:wesj)]

(In reply to JP Rosevear [:jpr] from comment #14)
> The implementation of save to pdf is here:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/
> browser.js#472
> 
> It appears that we use the download manager to fetch the page and then print
> it.  Is that expected to work?

We actually just "fake" the download so that users have a way to get to the saved page. The real work is just done by printing to a pdf file. But yes, it should work.

Comment 16

[Joe Drew (not getting mail)]

The images in the PDFs are black.

The best way to go forward here is to break in nsPrintEngine's constructor, say, and then break in RasterImage::Draw to find out where it's getting its faulty data.

Comment 18

[Kevin Brosnan [Ex-Mozilla]]

2011-11-04 good http://hg.mozilla.org/mozilla-central/rev/f3eea1384f14
2011-11-05 bad - http://hg.mozilla.org/mozilla-central/rev/a43602eadb2d 

This is inside the birch range so hg fromchange tochange does not work.

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

Those correspond to:
Good: http://hg.mozilla.org/users/dougt_mozilla.com/birch_native_ui_pre_wipe/rev/f3eea1384f14
Bad:  http://hg.mozilla.org/users/dougt_mozilla.com/birch_native_ui_pre_wipe/rev/a43602eadb2d

It looks like most of the changes between them are the merge from m-c

Comment 20

[Mark Finkle (:mfinkle) (use needinfo?)]

Kevin has verified that this bug is in XUL Fennec too. That means it is likely not a Maple related bug and we can use XUL Fennec to narrow the regression range.

Comment 21

[Mark Finkle (:mfinkle) (use needinfo?)]

We really need to fix this, but not blocking release.

Comment 22

[Aaron Train [:aaronmt]]

Future tracking?

Comment 23

[Martijn Wargers (dead)]

20111006 10.0a1 works fine
20111030 10.0a1 works fine
20111115 11.0a1 shows the bug

Comment 24

[Martijn Wargers (dead)]

Works in:
http://hg.mozilla.org/mozilla-central/rev/04b4ea333800
20111031

Fails in:
http://hg.mozilla.org/mozilla-central/rev/67d1049b0bf9
20111101

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2011-10-30+17%3A00%3A00&enddate=2011-11-01+01%3A00%3A00

I guess regression from bug 691061?

Comment 25

[Mark Finkle (:mfinkle) (use needinfo?)]

I can confirm that backing out bug 691061 restores "Save as PDF" functionality. The PDF has images rendered correctly.

Comment 26

[Mark Finkle (:mfinkle) (use needinfo?)]

Fixed by backing out bug 691061

Comment 27

[Martijn Wargers (dead)]

This works fine now on the Samsung Galaxy Nexus and the HTC Desire HD.
On the Samsung Galaxy SII, however, no images are shown when saving as PDF, but that doesn't appear to be a regression. I've filed bug 747782 for it.