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

Status: VERIFIED FIXED

(Core :: Print Preview, defect)

Description

[JK]

Bug 743252 removed the header and footer from PDFs. However, they are still shown in print preview.

Comment 1

[Alice0775 White]

Confirmed.
http://hg.mozilla.org/mozilla-central/rev/08a034e1d43a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130223 Firefox/22.0 ID:20130223031157

Comment 2

[Oskar Eisemuth]

Attachment: PrintPreview moznomarginboxes code

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

Comment 3

[Oskar Eisemuth]

Attachment: PrintPreview moznomarginboxes code fixed

Missed a line in patch file

Comment 4

[Zack Weinberg (:zwol)]

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.)

Comment 5

[Zack Weinberg (:zwol)]

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.

Comment 6

[Zack Weinberg (:zwol)]

Comment on attachment 8350886 [details] [diff] [review]
PrintPreview moznomarginboxes code fixed

https://hg.mozilla.org/integration/mozilla-inbound/rev/7418f487bb8b

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7418f487bb8b

Comment 8

[Oskar Eisemuth]

Attachment: Honor mozdisallowselectionprint and moznomarginboxes in print preview, use child document.

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.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8369195 [details] [diff] [review]
Honor mozdisallowselectionprint and moznomarginboxes in print preview, use child document.

https://hg.mozilla.org/integration/mozilla-inbound/rev/543e8c09e7b0

Is this something we could write a test for?

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/543e8c09e7b0

Comment 11

[Oskar Eisemuth]

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...

Comment 12

[Ryan VanderMeulen [:RyanVM]]

OK, thanks.

Comment 13

[Carsten Book [:Tomcat]]

correcting target milestone, sorry for the bugmail spam

Comment 14

[Zack Weinberg (:zwol)]

Filed follow-up bug 967105 for the difficulty of writing reftests for this sort of thing in general.

Comment 15

[Manuela Muntean [Away]]

Verified as fixed with Firefox 29 beta 1 on Win 7 x86 using the page provided in bug 743252: https://bugzilla.mozilla.org/attachment.cgi?id=714383