389358 - print headers do not display in print preview window

Status: RESOLVED FIXED

(Core :: Print Preview, defect, P3)

Description

[John Daggett (:jtd)]

Steps:

1. Open URL
2. File > Print Preview (with default Page Setup settings, Portrait, Shrink to Fit)

Result: print footer shows but not the print header

If the page is printed, the header prints but with a large top offset.

Comment 1

[Stuart Parmenter]

not sure what would have caused this...

Comment 2

[Daniel Holbert [:dholbert]]

I see this on linux as well.

OS --> all, assigning to me.

Comment 3

[Daniel Holbert [:dholbert]]

Testing Linux Trunk builds, this actually regressed way back in early 2006...

Regression window is between:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060329 Firefox/1.6a1
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060330 Firefox/1.6a1

Looks like a regression from Bug 331415: "Some printing code cleanup"

Comment 5

[Daniel Holbert [:dholbert]]

Ok, it looks like the print-preview header values (page title and page URL) were originally getting set in this function:

  nsSimplePageSequenceFrame::StartPrint(nsPresContext*   aPresContext,
                                        nsIPrintSettings* aPrintSettings,
                                        PRUnichar*        aDocTitle,
                                        PRUnichar*        aDocURL)
which was called via this callstack:

 #0  nsSimplePageSequenceFrame::StartPrint at nsSimplePageSequence.cpp:584
 #1  nsPrintEngine::DoPrint at nsPrintEngine.cpp:3295
 #2  nsPrintEngine::ShowDocListInternal at nsPrintEngine.cpp:4228
 #3  nsPrintEngine::ShowDocList at nsPrintEngine.cpp:4253
 #4  DocumentViewerImpl::InstallNewPresentation at nsDocumentViewer.cpp:4188
 #5  nsPrintEngine::FinishPrintPreview at nsPrintEngine.cpp:4337

However, the patch for bug 331415 (attachment 216680 [details] [diff] [review]) removed the ShowDocList and ShowDocListInternal functions, so after that patch, we only get down to level #4 in this callstack, and we never call nsSimplePageSequenceFrame::StartPrint.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: WIP #1

This restores the call to
   nsSimplePageSequenceFrame::StartPrint
which sets the fields
   mPageData->mDocTitle
   mPageData->mDocURL
on the nsSimplePageSequenceFrame object.

(Up to now, these fields weren't getting set during Print Preview, and this is why the header wasn't appearing)

I put the call inside of 
     nsPrintEngine::ReflowDocList
after we've reflowed the 

Concerns: 
 1) I'm not sure if this is the best place to put the call to StartPrint.
 2) I'm not sure we need all of the functionality in nsSimplePageSequenceFrame::StartPrint.  (I'm only calling that method because it's the only existing method which lets me set mPageData->mDocTitle and  mPageData->mDocURL)  Maybe I should just add a new method on nsSimplePageSequenceFrame & nsIPageSequenceFrame that just sets the doc title & URL, and does nothing else?

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to comment #6)
> I put the call inside of 
>      nsPrintEngine::ReflowDocList
> after we've reflowed the 

I meant to say "after we've reflowed the print object."

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 299853 [details] [diff] [review]
WIP #1

roc -- could you let me know if the WIP makes sense & if you've got any ideas on the two concerns listed in Comment 6?

Comment 9

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

I think since we were calling StartPrint before, we can depend on stuff like selection printing and page ranges not being enabled, so we can assume that code doesn't actually do anything.

Putting it there means we call it twice when really printing, don't we? I think the best place would be at the end of SetupToPrintContent, where we call PrintDocContent if we're really printing.

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: patch v2

> I think since we were calling StartPrint before, we can depend on stuff like
> selection printing and page ranges not being enabled, so we can assume that
> code doesn't actually do anything.

Ok, cool.

> I think
> the best place would be at the end of SetupToPrintContent, where we call
> PrintDocContent if we're really printing.

This patch puts it slightly earlier in that function, just after the call to BeginDocument if we were actually printing.  I put it there to take advantage of the fact that we've already got docTitleStr and docURLStr there, and we haven't freed them yet.

This patch only frees docTitleStr and docURLStr in the NON-print-preview case, because StartPrint() passes the strings by pointer, and so we need to keep those strings around for PrintPreview to use them.   They'll be freed later, in the nsSharedPageData's destructor:

  96 nsSharedPageData::~nsSharedPageData()
  97 {
 ...
 102   if (mDocTitle) nsMemory::Free(mDocTitle);
 103   if (mDocURL) nsMemory::Free(mDocURL);
 104 }

Comment 11

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

Comment on attachment 299888 [details] [diff] [review]
patch v2

+    // Print Preview -- pass docTitleStr and docURLStr down to seqFrame here.

"pass ownership of"

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: patch as landed

Improved comment -- here's patch as landed.

Comment 13

[Daniel Holbert [:dholbert]]

Checking in nsPrintEngine.cpp;
/cvsroot/mozilla/layout/printing/nsPrintEngine.cpp,v  <--  nsPrintEngine.cpp
new revision: 1.162; previous revision: 1.161
done


--> Fixed.

Comment 14

[Edward]

In tonight's build:

Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.9b4pre) Gecko/2008020702 SeaMonkey/2.0a1pre

the headers and footers are showing in the Print Preview window.  

However (please advise if this should be reported as a new bug), in Portrait mode, the headers were cut off at the top of the page and the footers did not print at all.  I suspect this has to do with the removal of margin settings under both Page Setup and Print/Preferences.  

This margin setting capability really needs to be restored.

Comment 15

[Edward]

I was able to get the footers to print, but only after editing about:config to use the same settings as SM 1.1.8.  

However some text is mixing in with the footers, which requires that scaling to be tested until it prints to the satisfaction of the user.  The item to ignore scaling and shrink to fit automatically, doesn't work, the scaling only seems to work with this box unchecked.