Closed Bug 389359 Opened 13 years ago Closed 12 years ago
jump-to-page in print preview goes to wrong position
Steps: 1. Open URL 2. File > Print Preview (with default Page Settings, portrait, shrink to fit) 3. Click on "Next Page" button Result: the last of five pages is displayed, page indicator shows 2. Entering "2" for the page value has the same effect, the last of five pages is shown. Dragging on the scroll bar correctly shows all five pages. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007072205 Minefield/3.0a7pre
Flags: blocking1.9? → blocking1.9+
This is worksforme. Can anyone reproduce?
Um, I've been told that is not WFM. Bad thing is that I can't reproduce this on latest nightly/windows, nor on Linux. So fixing this is pretty much impossible for me :(
Assignee: Olli.Pettay → nobody
i can reproduce this bug on Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b2pre) Gecko/2007111204 Minefield/3.0b2pre with the steps to reproduce from comment #0
Argh, sorry, my mistake. Did something stupid on windows. I can reproduce, but only on windows, which still makes this pretty impossible for me to fix.
Regression range is between the nightlies of Feb 13 and 14, 2007. Build IDs: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a3pre) Gecko/20070213 Minefield/3.0a3pre Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a3pre) Gecko/20070214 Minefield/3.0a3pre
This is actually a multi-platform bug -- it's not Windows only. We're just off by different amounts on different platforms. I tried this on both Windows and Linux: * Go to http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/ * Go into print-preview (which shows the document to be 7 pages long) * Type in a page number and press enter On Windows, if you try to jump to page 2, it takes you to the end of page 7. (i.e. it goes way too far) On Linux, if you try to jump to page 7, it takes you in between pages 5 and 6. (i.e. it doesn't go far enough) I'm guessing this has to do with the GetPrintPreviewScale() function added in Bug 369834 -- there's probably some place we need to use that function and scale our coordinates up or down.
OS: Windows XP → All
Summary: Clicking on next page button of print preview moves several pages ahead → jump-to-page in print preview goes to wrong position
(In reply to comment #7) > I'm guessing this has to do with the GetPrintPreviewScale() function Yup -- if I tweak this call to SetPrintPreviewScale in nsPrintEngine.cpp:1933 aPO->mPresContext->SetPrintPreviewScale(screenDPI / printDPI); and set the scale to be exactly 1.0 instead of the DPI ratio, then the next-page and jump-to-page actions work perfectly. (The page is at the wrong zoom factor, though, because of the changed scale.) So, that suggests there's something that needs to be scaled up/down by the PrintPreviewScale.
Assignee: nobody → dholbert
This fixes the issue by multiplying the argument of scrollableView->ScrollTo by the printPreviewScale factor. I've temporarily disabled subtracting the dead-space above the page, because on my linux box, deadSpaceGap is 1440 and yet there's no visible dead space there at all -- just the shadow of the previous page, and then the current page starts immediately where that shadow ends. (FWIW, on windows, there is a tiny amount of dead space, but it's much much less than in FF2, so that may also be incorrect.) I'll file a separate bug about this, with a note that we should reinstate deadSpaceGap-subtraction when that bug is fixed.
(In reply to comment #9) > I've temporarily disabled subtracting the dead-space above the page > ... > I'll file a separate bug about this, with a note that we should reinstate > deadSpaceGap-subtraction when that bug is fixed. Filed as bug 414075
So the deadSpaceGap subtraction didn't work even when there was no scale? And it doesn't work either inside or outside the parentheses for the multiplication?
Note that roc is really a better reviewer for this, since I haven't followed all the scaling stuff much, but the patch looks fine to me other than that.
(In reply to comment #11) > So the deadSpaceGap subtraction didn't work even when there was no scale? It did work at that point. However, Print-Preview had visible dead space on Linux at that point. (both before and after scale was added). Now, no dead space is visible on Linux. > And > it doesn't work either inside or outside the parentheses for the > multiplication? Correct. (I just re-tested both inside & outside, to make sure.) There's *zero* visible dead space in print-preview on Linux right now, so any subtraction there at all will make a chunk of the previous page visible. (This is filed as bug 414075, as mentioned in comment 10) (In reply to comment #12) > Note that roc is really a better reviewer for this, since I haven't followed > all the scaling stuff much, but the patch looks fine to me other than that. Ok -- CCing roc, and I'll ask him for review on the final patch. (gonna investigate bug 414075 a little bit first, to see if I can resolve the no-deadspace issue before landing this patch)
This patch just updates the previous patch's comment slightly to indicate where deadSpaceGap *should* be subtracted -- inside the parenthesis, before scaling -- if bug 414075 weren't broken. (I determined this by backing out the patch that caused bug 414075, and trying both positionings.)
Quick summary of patch: - Scale page-position by the PrintPreviewScale, so that we actually jump to the correct place. - Temporarily disable subtraction of deadSpaceGap in this jump, so that we jump to the exact top of the page, because deadSpaceGap is currently broken (bug 414075.)
Also, this patch works during zoomed print-preview (using ctrl+mousewheel) as well. (Thanks to smaug for telling me that zoomed print-preview exists. :))
Thanks for the review, roc! Patch v2 checked in. Checking in layout/base/nsDocumentViewer.cpp; /cvsroot/mozilla/layout/base/nsDocumentViewer.cpp,v <-- nsDocumentViewer.cpp new revision: 1.565; previous revision: 1.564 done Checking in layout/printing/nsPrintEngine.h; /cvsroot/mozilla/layout/printing/nsPrintEngine.h,v <-- nsPrintEngine.h new revision: 1.28; previous revision: 1.27 done
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.