Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Don't print the URL and other information when printing PDFs

RESOLVED FIXED in Firefox 20

Status

()

Core
Printing: Output
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: JK, Assigned: Julian Viereck)

Tracking

(Depends on: 3 bugs, {dev-doc-needed})

Trunk
mozilla22
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox19-, firefox20+ verified, firefox21+ verified, firefox22 verified)

Details

(Whiteboard: https://github.com/mozilla/pdf.js/issues/154 [pdfjs-c-feature] [pdfjs-d-printing])

Attachments

(3 attachments, 5 obsolete attachments)

(Reporter)

Description

5 years ago
The file name, URL, amount of pages, date and time should not be printed when printing PDFs with pdf.js. Adobe Reader doesn't print them.
(Reporter)

Updated

5 years ago
Blocks: 714712
The pdf page should also be printed on the full page.

Updated

5 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

5 years ago
Whiteboard: https://github.com/mozilla/pdf.js/issues/154

Updated

5 years ago
Whiteboard: https://github.com/mozilla/pdf.js/issues/154 → https://github.com/mozilla/pdf.js/issues/154 [pdfjs-c-feature] [pdfjs-d-printing]

Updated

5 years ago
OS: Windows 7 → All
Hardware: x86 → All

Comment 2

5 years ago
This issue shows up in feedback. I don't know why it got lost.

https://input.mozilla.org/opinion/3498048
https://input.mozilla.org/opinion/3495711

It sort of breaks the point of PDFs which is to have the printed file be exactly as it was intended.
tracking-firefox19: --- → ?

Comment 3

5 years ago
Triage doesn't feel this necessarily blocks the feature in FF19, but we'd like to hear how easily this may be resolved before making a final decision here. Any ideas Yury?
Assignee: nobody → ydelendik
tracking-firefox20: --- → +

Updated

5 years ago
Flags: needinfo?(ydelendik)
That directly connected with printing of the HTML/CSS content. I have no idea if it's hard or easy to extend the HTML/CSS with some special style attributes to suppress standard header/footer. There is also a possibility to extend the mozPrintCallback to return/set some special settings, but again, it is hard for me to judge is it's easy to add/test that in short period of time.
Flags: needinfo?(ydelendik)
Thanks for the feedback Yury, it looks to me like this might involve too much effort for the short time we have left until shipping 19 and at this point we really only want to land critical fixes to b6.  Will continue to track this for 20 so we can get it fixed and properly tested in earlier betas.
tracking-firefox19: ? → -

Updated

5 years ago
Assignee: ydelendik → nobody
(Assignee)

Updated

5 years ago
Duplicate of this bug: 839916
(Assignee)

Comment 7

5 years ago
How about implement a "mozNoPrintHeaderByDefault" and "mozNoPrintFooterByDefault" flag similar to the "mozdisallowselectionprint" flag implemented in bug #830278?

Some more thoughts are in: 

  https://bugzilla.mozilla.org/show_bug.cgi?id=839916#c0
http://www.w3.org/TR/css3-page/#margin-boxes has some features which could be used to override the default header/footer. However, there has been some discussion that maybe those could be replaced by something simpler.

A moznomarginboxes attribute might be better for now.
(Assignee)

Comment 9

5 years ago
Yury told me you're looking for someone to work on this, so I try to do the best I can for this week and maybe have to pass it to someone else afterwards.
Assignee: nobody → jviereck.dev
Status: NEW → ASSIGNED
(Assignee)

Comment 10

5 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
>
> A moznomarginboxes attribute might be better for now.

Okay - then let's drop my mozNoPrintHeaderByDefault and mozNoPrintFooterByDefault idea and go with mozNoMarginBoxes. 

Assume the <body> element has the mozNoMarginBoxes flag set and the page is printed. Is it still possible to choose the page header/footer from the print settings dialog?
Easiest would be to say "yes" --- only the default header/footer would be removed.
(Assignee)

Comment 12

5 years ago
Created attachment 713175 [details] [diff] [review]
WIP

WIP. My machine is still building so I cannot tell you if this is any good or not. Will test it more later today.
(Assignee)

Comment 13

5 years ago
There is a problem with the current patch aproach. The print settings implementation reuses the last made settings for the footer/header as the default value for next time's the printing. That means, if one page has the mozNoMarginBoxes attribute and printing is performed, then the user won't get the header/footers again for other pages without changing the settings again. This looks like an unexpected behavior for a user to me.

For setting the default values for the next print action in terms of footer/header, should we ignore the selections made for header/footer in case mozNoMarginBoxes is set?
That seems easiest.
(Assignee)

Comment 15

5 years ago
(In reply to Julian Viereck from comment #13)
>
> For setting the default values for the next print action in terms of
> footer/header, should we ignore the selections made for header/footer in
> case mozNoMarginBoxes is set?

My plan to implement this is the following:

- https://mxr.mozilla.org/mozilla-central/source/widget/nsIPrintSettings.idl: Add new getIgnoreMarginBoxOnWritePref() and setIgnoreMarginBoxOnWritePref(in bool ignore). If |ignoreMarginBoxOnWritePref| is set, then the header/footer information is not stored after the printing.
- https://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsPrintSettingsImpl.cpp: implement the new set/get functions
- nsPrintEngine.cpp: call `mPrt->mPrintSettings->setIgnoreMarginBoxOnWritePref(true)` if mozNoMarginBoxes is set
- https://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsPrintOptionsImpl.cpp#502: in `nsPrintOptions::WritePrefs`, skip all the `Preferences::SetString` that are related to footer/header if aPS->getIgnoreMarginBoxOnWritePref() is true.

Any idea for a better name then getIgnoreMarginBoxOnWritePref / setIgnoreMarginBoxOnWritePref is welcome :)
get/setPersistMarginBoxSettings?
(Assignee)

Comment 17

5 years ago
Created attachment 714382 [details] [diff] [review]
WIP 2

This patch implements all the functionality as discussed for the moznomarginboxes. It follows the implementation outline (except get/setPersistMarginBoxSettings was implemented as an attribute persistMarginBoxSettings). Pushing this one through the try server in a minute.
Attachment #713175 - Attachment is obsolete: true
(Assignee)

Comment 18

5 years ago
Created attachment 714383 [details]
Page I've used for testing.
(Assignee)

Comment 19

5 years ago
Created attachment 714396 [details] [diff] [review]
WIP 3

WIP 2 had rejects once I poped and pushed my queue again. Therefore I'm adding here the patch that I pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=86230226dffa
Attachment #714382 - Attachment is obsolete: true
(Assignee)

Comment 20

5 years ago
(In reply to Julian Viereck from comment #19)
> Created attachment 714396 [details] [diff] [review]
> WIP 3
> 
> WIP 2 had rejects once I poped and pushed my queue again. Therefore I'm
> adding here the patch that I pushed to try:
> https://tbpl.mozilla.org/?tree=Try&rev=86230226dffa

Try run looks good to me. I've only tested this on OSX so please can someone else download the binaries for other platforms and confirm that the header/footer are not printed by default?
(Assignee)

Updated

5 years ago
Attachment #714396 - Flags: review?(roc)
Attachment #714396 - Attachment is patch: true
Comment on attachment 714396 [details] [diff] [review]
WIP 3

Review of attachment 714396 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with those fixed.

::: widget/xpwidgets/nsPrintOptionsImpl.cpp
@@ +615,5 @@
>            Preferences::SetBool(GetPrefName(kPrintOddPages, aPrinterName), b);
>          }
>    }
>  
> +  if (aFlags & nsIPrintSettings::kInitSaveHeaderLeft && persistMarginBoxSettings) {

Just put this entire code block inside if (persistMarginBoxSettings).

::: widget/xpwidgets/nsPrintSettingsImpl.cpp
@@ +367,5 @@
> +/* attribute boolean persistMarginBoxSettings; */
> +NS_IMETHODIMP nsPrintSettings::GetPersistMarginBoxSettings(bool *aPersistMarginBoxSettings)
> +{
> +  NS_ENSURE_ARG_POINTER(aPersistMarginBoxSettings);
> +  *aPersistMarginBoxSettings = (bool)mPersistMarginBoxSettings;

Unnecessary cast.

@@ +372,5 @@
> +  return NS_OK;
> +}
> +NS_IMETHODIMP nsPrintSettings::SetPersistMarginBoxSettings(bool aPersistMarginBoxSettings)
> +{
> +  mPersistMarginBoxSettings = (bool)aPersistMarginBoxSettings;

Unnecessary cast.
Attachment #714396 - Flags: review?(roc) → review+
(Assignee)

Comment 22

5 years ago
Created attachment 714887 [details] [diff] [review]
WIP 4

Addresses the review comments made by :roc in comment #21. Carrying over r+.
Attachment #714396 - Attachment is obsolete: true
Attachment #714887 - Flags: review+
(Assignee)

Comment 23

5 years ago
Created attachment 714888 [details] [diff] [review]
Disable header/footer when printing using PDF Viewer

This sets the |moznomarginboxes| for the PDF Viewer.
(Assignee)

Comment 24

5 years ago
Requesting "verifyme" as I've only test this on my OSX/10.8 machine and haven't done testing on other platforms.
Keywords: verifyme

Comment 25

5 years ago
According to http://boomswaggerboom.wordpress.com/2013/02/19/exciting-stuff-firefox-19s-built-in-pdf-reader/#comment-20789 we're also scaling the page. Can we not do that either?

Comment 26

5 years ago
(In reply to [:Cww] from comment #25)
> [...] we're also scaling the page. Can we not
> do that either?

There really should be a separate Page Setup for PDF with these defaults: scaling factor "Shrink to Fit" (paper size), no margins, no headers and footers. Changes to these settings should be possible on a per print job basis.
(Assignee)

Comment 27

5 years ago
(In reply to Heribert Slama from comment #26)
> There really should be a separate Page Setup for PDF with these defaults:
> scaling factor "Shrink to Fit" (paper size), no margins, no headers and
> footers. Changes to these settings should be possible on a per print job
> basis.

Thanks a lot raising this. Can you please open a separate bug for this? Getting the page size handling right will require some more work and specs to be implemented. That's why I don't want it to be covered in this bug here.
(Assignee)

Comment 28

5 years ago
Created attachment 716024 [details] [diff] [review]
WIP 4.1

Same as WIP 4 but with patch comment & author.
Attachment #714887 - Attachment is obsolete: true
(Assignee)

Comment 29

5 years ago
Comment on attachment 716024 [details] [diff] [review]
WIP 4.1

Carry over r+.
Attachment #716024 - Flags: review+
(Assignee)

Comment 30

5 years ago
Created attachment 716026 [details] [diff] [review]
Disable header/footer when printing using PDF Viewer

Same as before but with patch comment & author.
Attachment #714888 - Attachment is obsolete: true
(Assignee)

Comment 31

5 years ago
I've tested the try-run build on a windows machine and it worked. Therefore removing |verifyme| and added |checkin-needed|.
Keywords: verifyme → checkin-needed
Attachment #716024 - Attachment is obsolete: true
Attachment #716024 - Attachment is obsolete: false
https://hg.mozilla.org/integration/mozilla-inbound/rev/8de609c5d378
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ab6e6416d67

I assume that the PDF Viewer patch will be getting upstreamed as well?
Keywords: checkin-needed
Component: PDF Viewer → Printing: Output
Product: Firefox → Core
(Assignee)

Comment 33

5 years ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #32)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/8de609c5d378
> https://hg.mozilla.org/integration/mozilla-inbound/rev/8ab6e6416d67
> 
> I assume that the PDF Viewer patch will be getting upstreamed as well?

Landed already on the GitHub repo: https://github.com/mozilla/pdf.js/pull/2734
https://hg.mozilla.org/mozilla-central/rev/8de609c5d378
https://hg.mozilla.org/mozilla-central/rev/8ab6e6416d67
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
(Reporter)

Updated

5 years ago
Depends on: 844473
(Assignee)

Comment 35

5 years ago
I've created try builds with this patch applyed on Aurora and Beta release channel:

https://tbpl.mozilla.org/?tree=Try&rev=c3b93601e804
https://tbpl.mozilla.org/?tree=Try&rev=72f30589cb30

Both try runs fail on Android. Any idea why?
(In reply to Julian Viereck from comment #35)
> I've created try builds with this patch applyed on Aurora and Beta release
> channel:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=c3b93601e804
> https://tbpl.mozilla.org/?tree=Try&rev=72f30589cb30
> 
> Both try runs fail on Android. Any idea why?

CCing mfinkle/blassey, who have a lot of experience here. I think try server has issues against Aurora/Beta.
Once it's determined what the cause of the Android failures are, please nominate for uplift to branches.
status-firefox20: --- → affected
status-firefox21: --- → affected
status-firefox22: --- → fixed
tracking-firefox21: --- → +

Updated

5 years ago
Duplicate of this bug: 843217
(Assignee)

Comment 39

5 years ago
Gave the patch another try-run, but still burning on Android: https://tbpl.mozilla.org/?tree=Try&rev=aa8827708fd4
Mark,
Is android on try for aurora/beta permanently broken or is this some issue with the patch?
Flags: needinfo?(mark.finkle)
try is generally unreliable for non-mozilla-central branches, since its slaves use mozilla-central configurations. I think this is particularly true for mobile given bundle name differences and such. So I wouldn't block landing any patches there on try results - you need to monitor your pushes to those repos anyways.
Duplicate of this bug: 846819
Gavin's comment 41 covers it all
Flags: needinfo?(mark.finkle)
(In reply to [:Cww] from comment #25)
> According to
> http://boomswaggerboom.wordpress.com/2013/02/19/exciting-stuff-firefox-19s-
> built-in-pdf-reader/#comment-20789 we're also scaling the page. Can we not
> do that either?

(In reply to Julian Viereck from comment #27)
> (In reply to Heribert Slama from comment #26)
> > There really should be a separate Page Setup for PDF with these defaults:
> > scaling factor "Shrink to Fit" (paper size), no margins, no headers and
> > footers. Changes to these settings should be possible on a per print job
> > basis.
> 
> Thanks a lot raising this. Can you please open a separate bug for this?
> Getting the page size handling right will require some more work and specs
> to be implemented. That's why I don't want it to be covered in this bug here.

I've filed bug 847049 for the downsizing/scaling issue. Translating the suggested-above understanding of "Shrink to fit" (paper size): We should use original size whereever possible unless that's bigger than the paper, then shrink to paper size without adding margins or whatsoever.

Setting dependency for ease of tracking (and might be technically related).
Depends on: 847049
(Assignee)

Comment 45

4 years ago
Comment on attachment 716024 [details] [diff] [review]
WIP 4.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: UA's default print header/footer show up on top of PDF content
Testing completed (on m-c, etc.): yes - landed on m-c a few days back and sticked
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #716024 - Flags: approval-mozilla-beta?
Attachment #716024 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 46

4 years ago
Comment on attachment 716026 [details] [diff] [review]
Disable header/footer when printing using PDF Viewer

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: UA's default print header/footer show up on top of PDF content
Testing completed (on m-c, etc.): yes - landed on m-c a few days back and sticked
Risk to taking this patch (and alternatives if risky): very low risk
String or UUID changes made by this patch: none
Attachment #716026 - Flags: approval-mozilla-beta?
Attachment #716026 - Flags: approval-mozilla-aurora?
Depends on: 748935
Comment on attachment 716024 [details] [diff] [review]
WIP 4.1

low risk uplift of a recent feature in Fx19.This helps with avoiding printing of header/footer which is the expected behavior.

Please make sure to land it before 3/5(tomorrow) for this to make it into FX 20 beta 3.
Attachment #716024 - Flags: approval-mozilla-beta?
Attachment #716024 - Flags: approval-mozilla-beta+
Attachment #716024 - Flags: approval-mozilla-aurora?
Attachment #716024 - Flags: approval-mozilla-aurora+

Updated

4 years ago
Attachment #716026 - Flags: approval-mozilla-beta?
Attachment #716026 - Flags: approval-mozilla-beta+
Attachment #716026 - Flags: approval-mozilla-aurora?
Attachment #716026 - Flags: approval-mozilla-aurora+

Updated

4 years ago
Keywords: checkin-needed
Duplicate of this bug: 847824
https://hg.mozilla.org/releases/mozilla-aurora/rev/a5167835c4b5
https://hg.mozilla.org/releases/mozilla-aurora/rev/171bd25178c6

Pushed to Aurora. However, this requires binary approval to land on Beta due to the IDL changes. I'll land this on beta once it gets approval.
status-firefox21: affected → fixed
Keywords: checkin-needed
(In reply to Ryan VanderMeulen [:RyanVM] from comment #49)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/a5167835c4b5
> https://hg.mozilla.org/releases/mozilla-aurora/rev/171bd25178c6
> 
> Pushed to Aurora. However, this requires binary approval to land on Beta due
> to the IDL changes. I'll land this on beta once it gets approval.

Ryan, thanks for pushing this to Aurora. How can we request the binary approval required for Beta?
Flags: needinfo?(ryanvm)
302 jorgev :)
Flags: needinfo?(ryanvm)
It's only an addition to an existing interface, and doesn't look like something binary add-ons would normally use.

ba+ from me.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #49)
> Pushed to Aurora. However, this requires binary approval to land on Beta due
> to the IDL changes. I'll land this on beta once it gets approval.

(In reply to Jorge Villalobos [:jorgev] from comment #52)
> It's only an addition to an existing interface, and doesn't look like
> something binary add-ons would normally use.
> 
> ba+ from me.

Ryan, with :jorgev's ba+, this is now waiting for you to land it on beta asap :)
Flags: needinfo?(ryanvm)
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-beta/rev/7675ab257ed4
https://hg.mozilla.org/releases/mozilla-beta/rev/221b9038b2a7
status-firefox20: affected → fixed
Flags: needinfo?(ryanvm)
Keywords: checkin-needed
Depends on: 848641
Keywords: dev-doc-needed
(Assignee)

Comment 55

4 years ago
For reference: I'm working on a proper standard to disable the header/footer here: http://lists.w3.org/Archives/Public/www-style/2013Mar/0060.html

Removing keyword "dev-doc-needed" as I don't think there is value in documenting this flag. I hope there will be a standardized solution soon that might be different from what we have here at the moment. Please feel free to re-add the flag if you disagree with me.
Keywords: dev-doc-needed
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130307 Firefox/21.0

Verified as fixed on Firefox 20 Beta 4 (buildID: 20130307075451) and latest Aurora (buildID: 20130307042016). The URL and other information are not printed in Header/Footer by default.
status-firefox20: fixed → verified

Updated

4 years ago
Depends on: 853033
Jorge, this just came up in the context of bug 853033 (because this didn't include a required UUID change). This seems like the kind of interface change that we would automatically reject on beta and should not have recevied ba+.

"It's only an addition to an existing interface" doesn't mean much; any change to interfaces can affect binary compat.
Flags: needinfo?(jorge)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #57)
> Jorge, this just came up in the context of bug 853033 (because this didn't
> include a required UUID change). This seems like the kind of interface
> change that we would automatically reject on beta and should not have
> recevied ba+.
> 
> "It's only an addition to an existing interface" doesn't mean much; any
> change to interfaces can affect binary compat.

I know any changes can affect binary compatibility, but this didn't appear to be something that add-ons would normally use, which is why I approved it. I didn't notice the missing UUID rev, though :(. You're right that I should be more conservative with these approvals.

The comment about just adding to the interface was meant to address non-binary compatibility.
Flags: needinfo?(jorge)
(Assignee)

Comment 59

4 years ago
(In reply to Julian Viereck from comment #55)
> For reference: I'm working on a proper standard to disable the header/footer
> here: http://lists.w3.org/Archives/Public/www-style/2013Mar/0060.html
> 
> Removing keyword "dev-doc-needed" as I don't think there is value in
> documenting this flag. I hope there will be a standardized solution soon
> that might be different from what we have here at the moment. Please feel
> free to re-add the flag if you disagree with me.

Not sure if there will be a standard for hiding/showing the header soon, so I re-add the "dev-docs-needed" flag for now.
Keywords: dev-doc-needed

Comment 60

4 years ago
Verified as fixed on Windows 7, Ubuntu 12.04 and Mac OSX 10.8.3, on Firefox 21 beta 6 (20130430204233).
status-firefox21: fixed → verified

Comment 61

4 years ago
Also verified as fixed on Firefox 22 beta 6 - 20130617145905 - same OSs as comment 60.
status-firefox22: fixed → verified

Updated

4 years ago
Depends on: 967105

Updated

4 years ago
No longer depends on: 967105
Depends on: 1197647
Now that we have a more standard conform way to archive the same effect as with mozNoMarginBoxes (see https://bugzilla.mozilla.org/show_bug.cgi?id=1260480) I suggested to remove this feature in https://bugzilla.mozilla.org/show_bug.cgi?id=1260480.
Sorry, first link was meant to be https://bugzilla.mozilla.org/show_bug.cgi?id=1250674.
You need to log in before you can comment on or make changes to this bug.