Closed Bug 844473 Opened 12 years ago Closed 11 years ago

Don't show the header and footer in print preview of PDFs


(Core :: Print Preview, defect)

Windows 7
Not set





(Reporter: spammaaja, Assigned: dev_oskar)


(Blocks 1 open bug)



(1 file, 2 obsolete files)

Bug 743252 removed the header and footer from PDFs. However, they are still shown in print preview.
Blocks: 743252
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130223 Firefox/22.0 ID:20130223031157
Ever confirmed: true
The patch simple copies the logic of the print code, so both are sync.

Changing the footer and header in the ui doesn't work anyway with moznomarginboxes. At least now the printpreview and printout are the same, and the PrintPreview is affected by user changes for footers and headers
Missed a line in patch file
Attachment #8350885 - Attachment is obsolete: true
Blocks: 947125
Comment on attachment 8350886 [details] [diff] [review]
PrintPreview moznomarginboxes code fixed

(bringing conversation over from bug 650966)

> When Bug 844473 doesn't even get attention I have no idea how I can help, if
> even simple stuff won't get fixed or at least a reason why not.

You have tripped over a long-standing problem with our process.  If you just post a patch to a bug, no one will notice.  You have to select a specific person to ask for a code review.  Worse, there is no automatic or even fully reliable way to know who understands the code well enough to review *and* is procedurally entitled to do so.  Even worse, there is a lot of code understood *only* by a small handful of chronically-overworked "super-reviewers", or in some cases, not truly understood by anyone at all.  (Be glad you aren't trying to patch the table layout engine. :-/ )

In this case, based on `hg annotate` for this area of nsDocumentViewer.cpp, I have decided that Robert O'Callahan is at least plausibly appropriate as the reviewer.  I can also confirm that my own (limited) understanding of this code says that this is the correct change.  (That's what 'feedback+' means.)
Attachment #8350886 - Flags: review?(roc)
Attachment #8350886 - Flags: feedback+
FYI, once you get a positive review from roc (or whoever he reassigns it to), you can then go ahead and push the patch to mozilla-inbound.  If you don't have access to do that yet, you instead put "checkin-needed" in the "keywords" box at the top of the bug and someone will do it for you.
Assignee: nobody → dev_oskar
Comment on attachment 8350886 [details] [diff] [review]
PrintPreview moznomarginboxes code fixed
Attachment #8350886 - Flags: checkin? → checkin+
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
With the first PrintPreview call mDocument is likely not the document that belongs to aChildDOMWin. The patch switches from mDocument to doc of aChildDOMWin. (as used by the mPrintEngine->Initialize call)
Additional it adds debug log, as the manipulation of header footer settings aren't saved anymore. (The current saving code for this is broken anyway)

Technical nsIPrintSettings should gain a new setting for enableMarginBoxes and the code for moznomarginboxes rewritten to change this setting once when starting printPreview so the user can control this by ui, I will create a new bug for this.
Attachment #8350886 - Attachment is obsolete: true
Attachment #8369195 - Flags: review?(roc)
Resolution: FIXED → ---
Target Milestone: mozilla29 → ---
Comment on attachment 8369195 [details] [diff] [review]
Honor mozdisallowselectionprint and moznomarginboxes in print preview, use child document.

Is this something we could write a test for?
Attachment #8369195 - Flags: checkin? → checkin+
Flags: needinfo?(dev_oskar)
Flags: in-testsuite?
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Hmm, creating a test is quite complicate.

moznomarginboxes influences the print settings when Print or PrintPreview gets called. (in a very odd way, and only then)

Reftest assumes currently fixed settings, and directly uses setPageMode(true, ps).
So header and footer would need to be enabled for the first part and PrintPreview mode somehow activated.
For the reference page the settings would need to be reset to zero, and again PrintPreview mode activated, then comparing both. 

Not sure how to write such code...
Flags: needinfo?(dev_oskar)
OK, thanks.
Flags: in-testsuite? → in-testsuite-
correcting target milestone, sorry for the bugmail spam
Target Milestone: mozilla30 → mozilla29
Blocks: 967105
No longer blocks: 967105
Filed follow-up bug 967105 for the difficulty of writing reftests for this sort of thing in general.
Keywords: verifyme
Verified as fixed with Firefox 29 beta 1 on Win 7 x86 using the page provided in bug 743252:
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.