Closed
Bug 307404
Opened 19 years ago
Closed 19 years ago
respect the paper size setting
Categories
(Core :: Printing: Output, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: chpe, Assigned: chpe)
References
Details
Attachments
(1 file, 1 obsolete file)
2.95 KB,
patch
|
kherron+mozilla
:
review+
roc
:
superreview+
roc
:
approval-branch-1.8.1-
dveditz
:
approval1.8.0.1-
|
Details | Diff | Splinter Review |
I'm trying to use nsIPrintSettings' PaperWidth and PaperHeight properties to set the paper size to use for the print output. But nsPostScriptObj doesn't use those settings; instead it uses the *paper name* setting to look the paper size up [see http://lxr.mozilla.org/mozilla/source/gfx/src/ps/nsPostScriptObj.cpp#302] in its extremely limited set of only 6 paper sizes [http://lxr.mozilla.org/mozilla/source/gfx/src/psshared/nsPaperPS.cpp#46] ! The attached patch fixes that by getting the paper width/height properties from the nsIDeviceContextSpecPS only if the paper name wasn't found in the list. I chose this approach because it doesn't change anything if you only set the paper name and expect the size to be chosen automatically (which may well be what existing apps and embedders could rely on). Not the same as bug 306653, but could serve as backend for that bug's frontend changes, instead of the nsPostScriptObj.cpp change in that bug's attachment 194624 [details] [diff] [review].
Assignee | ||
Comment 1•19 years ago
|
||
Chosing reviewer based on reviewers of recent checkins to nsPostScriptObj.cpp, hope that's correct.
Assignee: printing → chpe
Status: NEW → ASSIGNED
Attachment #195164 -
Flags: review?(kherron+mozilla)
Comment 2•19 years ago
|
||
Comment on attachment 195164 [details] [diff] [review] proposed fix I like this, but I'd like to go further. nsPostScriptObj uses the paper name to look up a size because it's always worked that way, not out of necessity. If the device context spec has a call to return the paper size, it'd be cleaner to use that every time rather than consult the paper size list. This would probably help with bug 272670 as well. Christian, would you be able to rework this patch?
Attachment #195164 -
Flags: review?(kherron+mozilla) → review-
Assignee | ||
Comment 3•19 years ago
|
||
I removed the paper size lookup by name and get the size from the print settings instead. (I still get the paper name though, since it's written into the PS output.) I've checked that both the toolkit and xpfe print dialogues set the paper width/height, see http://lxr.mozilla.org/seamonkey/source/toolkit/components/printing/content/printjoboptions.js#916 and http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/unix/printjoboptions.js#914 . It looks like this might indeed fix bug 272670 too.
Attachment #195164 -
Attachment is obsolete: true
Attachment #195647 -
Flags: review?(kherron+mozilla)
Comment 4•19 years ago
|
||
Comment on attachment 195647 [details] [diff] [review] reworked patch This looks good. Thanks for the update.
Attachment #195647 -
Flags: superreview?(roc)
Attachment #195647 -
Flags: review?(kherron+mozilla)
Attachment #195647 -
Flags: review+
Be aware, that you should not use the paper size itself, but the printible area. Most laser/ink printers have unprintible areas, where the source/page number etc. is printed on at now
Assignee | ||
Comment 6•19 years ago
|
||
(In reply to comment #5) > Be aware, that you should not use the paper size itself, but the printible area. > Most laser/ink printers have unprintible areas, where the source/page number > etc. is printed on at now nsPaper (what the code is using before this patch to get the paper size) does not take that into account either, see http://lxr.mozilla.org/seamonkey/source/gfx/src/psshared/nsPaperPS.cpp#49 .
Attachment #195647 -
Flags: superreview?(roc) → superreview+
Comment 7•19 years ago
|
||
Checked in: /cvsroot/mozilla/gfx/src/ps/nsPostScriptObj.cpp,v <-- nsPostScriptObj.cpp new revision: 1.125; previous revision: 1.124 Thanks for the patch, Christian!
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 8•19 years ago
|
||
Comment on attachment 195647 [details] [diff] [review] reworked patch Requesting approval to check on the 1.8 branch. This patch fixes the symptoms of bug 272670 and bug 308895.
Attachment #195647 -
Flags: approval1.8b5?
Comment 9•19 years ago
|
||
can we get a trunk verification on this before we consider it for the branch? Thanks.
Comment 10•19 years ago
|
||
(In reply to comment #9) > can we get a trunk verification on this before we consider it for the branch? > Thanks. https://bugzilla.mozilla.org/show_bug.cgi?id=308895#c10 https://bugzilla.mozilla.org/show_bug.cgi?id=272670#c17
Status: RESOLVED → VERIFIED
Comment 11•19 years ago
|
||
Comment on attachment 195647 [details] [diff] [review] reworked patch I don't think this patch is bad per se, but we it's apparent we have a problem with the print dialog returning bad data bug 272670 and bug 309988) and this patch in conjunction with that seems to be causing print failures.
Attachment #195647 -
Flags: approval1.8b5?
Comment 12•19 years ago
|
||
Did this cause bug 315687?
Comment 13•18 years ago
|
||
Comment on attachment 195647 [details] [diff] [review] reworked patch I think we need to get this onto at least the 1.8 branch if not the 1.8.0 branch - probably affects non-letter size users more than letter size users.
Attachment #195647 -
Flags: approval1.8.1?
Attachment #195647 -
Flags: approval1.8.0.1?
Comment 14•18 years ago
|
||
Comment on attachment 195647 [details] [diff] [review] reworked patch too late for 1.8.0.1, minusing. I'm not going to nominate for 1.8.0.2 because I suspect it would just get rejected -- comments 11 and 12 imply unfixed regressions and those issues would have to be addressed.
Attachment #195647 -
Flags: approval1.8.0.1? → approval1.8.0.1-
Updated•18 years ago
|
Attachment #195647 -
Flags: approval1.8.1? → branch-1.8.1?(roc)
Comment on attachment 195647 [details] [diff] [review] reworked patch We need to sort out the regressions before taking this on any branch.
Attachment #195647 -
Flags: approval-branch-1.8.1?(roc) → approval-branch-1.8.1-
You need to log in
before you can comment on or make changes to this bug.
Description
•