Closed Bug 414075 Opened 15 years ago Closed 15 years ago
Little or no dead space between pages in Print Preview
In Trunk on Linux, there's no visible dead space above the first page, between pages, and after the last page. (There *are* page shadows in between the pages and after the last page, but no dead white space.) On Windows, there's some dead white space, but it's very little -- much less than there was in Firefox 2. I'm pretty sure there should be *some* dead white space, because a) there was in Firefox 2 b) there is a small amount on Windows c) the DocumentViewerImpl::PrintPreviewNavigate code seems to assume that there is -- it jumps to the y-position of the given page, minus the deadSpaceGap, to give the page a whitespace border above it.* *Note -- the "minus the deadSpaceGap" part of DocumentViewerImpl::PrintPreviewNavigate may be temporarily disabled as part of the fix for bug 389359, because of this bug here. (see proposed patch, attachment 299323 [details] [diff] [review]) When this bug here is fixed, we should re-enable this subtraction.
I'm assuming that the intended behavior is for there to be as much dead white space as there was in Firefox 2 -- correct me if that's wrong.
Actually, in Trunk, there's also no dead space on the sides of the page, either. (Firefox 2 had some space to the sides, for a nice padding effect.)
On linux, the dead space disappeared between these builds: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007112004 Minefield/3.0b2pre Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007112105 Minefield/3.0b2pre --> regression from bug 404519, probably.
(In reply to comment #5) > --> regression from bug 404519, probably. Yup -- backing out the bug 404519 patch from current trunk fixes this bug here, reinstating some dead space on linux in PrintPreview. Also, I found one more reason there should definitely be visible dead space- -- nsSimplePageSequenceFrame::GetDeadSpaceValue returns NS_INCHES_TO_TWIPS(0.25), and has since 2002. (see URL below) http://mxr.mozilla.org/seamonkey/source/layout/generic/nsSimplePageSequence.h#106 So we should be seeing 1/4 inch of dead space.
Requesting blocking, since this is a pretty visible regression from Firefox 2.0. (see screenshots)
Please try this patch before backing out Bug 404519...
Actually, this is a better fix. Reuse gapInTwips.
I can confirm that "better fix" reinstates the 1/4 inch of dead white space. Yay! Thanks for fixing this, Jeremy. One thing you should add as part of this bug's patch, though: Now that deadSpaceGap is correct again, can you subtract it inside the parenthesis in the computation of "newYPosn" at nsDocumentViewer.cpp:3669, and remove the XXXdholbert comment there about this issue? It's at this point in the code http://lxr.mozilla.org/mozilla/source/layout/base/nsDocumentViewer.cpp#3669 and we need to change 3667 nscoord newYPosn = 3668 nscoord(mPrintEngine->GetPrintPreviewScale() * 3669 float(fndPageFrame->GetPosition().y)); to 3667 nscoord newYPosn = 3668 nscoord(mPrintEngine->GetPrintPreviewScale() * 3669 float(fndPageFrame->GetPosition().y - deadSpaceGap)); and remove the comment above that chunk. (this is new code as of yesterday, checked in as part of bug 389359)
Actually, maybe cancel my last comment -- after making the change I mentioned above, we're not quite lining up correctly on Jump-To-Page -- a little bit of of the previous page is visible, in addition to the (expected) dead space. Investigating this a bit...
Ok -- I think there are more issues involved with the deadSpaceGap subtraction issue in print-preview jump-to-page functionality, so we should leave that disabled for now, and I'll file a separate bug for that. This bug's patch should land as-is. (ignoring the changes I suggested in comment 10)
Comment on attachment 300540 [details] [diff] [review] Better fix... Requesting approval for this patch to go into beta 3. Justification: - Makes print-preview prettier, fixes visible regression. (see screenshots) - Minimal change, with little (or no) risk - The replaced code was clearly a typo, and the added code does what was clearly intended to be done
Attachment #300540 - Flags: approval1.9b3?
(In reply to comment #12) > Ok -- I think there are more issues involved with the deadSpaceGap subtraction > issue in print-preview jump-to-page functionality, so we should leave that > disabled for now, and I'll file a separate bug for that. Filed as bug 415012.
Comment on attachment 300540 [details] [diff] [review] Better fix... Go for it
Attachment #300540 - Flags: approval1.9b3? → approval1.9b3+
Patch checked in. Thanks for the patch, Jeremy! Checking in nsSimplePageSequence.cpp; /cvsroot/mozilla/layout/generic/nsSimplePageSequence.cpp,v <-- nsSimplePageSequence.cpp new revision: 3.163; previous revision: 3.162 done
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.