Closed Bug 743252 Opened 13 years ago Closed 12 years ago

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

Categories

(Core :: Printing: Output, defect)

Product:
Core
Component:
Printing: Output
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla22
Tracking Flags:
Tracking Status
firefox19 - ---
firefox20 + verified
firefox21 + verified
firefox22 --- verified

People

(Reporter: spammaaja, Assigned: julian.viereck)

References

(Depends on 1 open bug)

Details

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

Attachments

(3 files, 5 obsolete files)

Page I've used for testing.
(deleted), text/html
Details
WIP 4.1
(deleted), patch
julian.viereck
: review+
bajaj
: approval-mozilla-aurora+
bajaj
: approval-mozilla-beta+
Details | Diff | Splinter Review
Disable header/footer when printing using PDF Viewer
(deleted), patch
bajaj
: approval-mozilla-aurora+
bajaj
: approval-mozilla-beta+
Details | Diff | Splinter Review
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.
Blocks: 714712
The pdf page should also be printed on the full page.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: https://github.com/mozilla/pdf.js/issues/154
Whiteboard: https://github.com/mozilla/pdf.js/issues/154 → https://github.com/mozilla/pdf.js/issues/154 [pdfjs-c-feature] [pdfjs-d-printing]
OS: Windows 7 → All
Hardware: x86 → All
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: --- → ?
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: --- → +
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: ? → -
Assignee: ydelendik → nobody
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.
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
(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.
Attached patch WIP (obsolete) (deleted) — — Splinter Review
WIP. My machine is still building so I cannot tell you if this is any good or not. Will test it more later today.
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?
(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?
Attached patch WIP 2 (obsolete) (deleted) — — Splinter Review
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
Attached file Page I've used for testing. (deleted) —
Attached patch WIP 3 (obsolete) (deleted) — — Splinter Review
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
(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?
Attachment #714396 - Flags: review?(roc)
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+
Attached patch WIP 4 (obsolete) (deleted) — — Splinter Review
Addresses the review comments made by :roc in comment #21. Carrying over r+.
Attachment #714396 - Attachment is obsolete: true
Attachment #714887 - Flags: review+
Attached patch Disable header/footer when printing using PDF Viewer (obsolete) (deleted) — — Splinter Review
This sets the |moznomarginboxes| for the PDF Viewer.
Requesting "verifyme" as I've only test this on my OSX/10.8 machine and haven't done testing on other platforms.
Keywords: verifyme
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 [: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.
(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.
Attached patch WIP 4.1 (deleted) — — Splinter Review
Same as WIP 4 but with patch comment & author.
Attachment #714887 - Attachment is obsolete: true
Comment on attachment 716024 [details] [diff] [review] WIP 4.1 Carry over r+.
Attachment #716024 - Flags: review+
Same as before but with patch comment & author.
Attachment #714888 - Attachment is obsolete: true
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
Component: PDF Viewer → Printing: Output
Product: Firefox → Core
(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
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Depends on: 844473
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: --- → +
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.
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
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?
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+
Attachment #716026 - Flags: approval-mozilla-beta?
Attachment #716026 - Flags: approval-mozilla-beta+
Attachment #716026 - Flags: approval-mozilla-aurora?
Attachment #716026 - Flags: approval-mozilla-aurora+
Keywords: checkin-needed
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
Depends on: 848641
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
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)
(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
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
Also verified as fixed on Firefox 22 beta 6 - 20130617145905 - same OSs as comment 60.
status-firefox22: fixed → verified
Depends on: 967105
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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: