430150 - Print-selection output too high (or off of page) due to print scaling

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Daniel Holbert [:dholbert]]

When we're doing a print-selection, we shift the selected area up to the top of the printed page.  However, when print scaling[1] is involved, it looks like we don't correctly scale our units when we shift the selected area.  Consequently, the selected area gets shifted too far up, possibly all the way off the page.

This is demonstrated by the upcoming testcases.

STEPS TO REPRODUCE:
 1. Load testcase
 2. Select the line of text
 3. Print selection

EXPECTED RESULT:
 Selected text is at top of page, inside the margins.
(and inside the headers, if you've got the patch for bug 429337 applied and can see print headers)
 
ACTUAL RESULT:
 Varies by testcase:
   - in testcase 1, the selected text appears at the very top edge of the page (outside the print header, if you've got the patch for bug 429337 applied.)
   - in testcase 2, the selected text isn't visible. (it's off the page)

[1] "print scaling" is when our content is too wide to fit on the page, and so we shrink the whole page by some scaling factor in order to make it fit.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

This testcase uses a 9in-wide div to trigger print-scaling.

The div is 1in tall, which means that (ignoring scaling) the selected text will need to be shifted up 1in to be at the top of the page, when printed with print-selection.

However, the text actually ends up being placed near the absolute top edge of the page, in the margin region, which is higher than it should be.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: reference 1

This reference case uses a 4in wide div, instead of 9in wide, so it doesn't trigger print scaling.

As a result, we get expected behavior.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: testcase 2

This is like testcase 1, but it uses a div that's much taller, which means the selected text needs to be shifted by a lot more.

This means our scaling error has a much bigger effect, shifting the text all the way off of a printed page.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: reference 2

This reference case just uses a skinnier div than testcase 2, so that we don't need to do any print scaling.

Comment 5

[Daniel Holbert [:dholbert]]

This bug is a regression WRT Firefox 2. (Firefox 2 gives expected output when I apply the steps from comment 0.)

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: fix v1

This patch fixes the bug by appropriately scaling the selection position & height, in the call to  pageSequence->SetSelectionHeight.

This fixes all testcases on this bug and its duplicates.  I verified it on the following testcases:
 - testcases 1 and 2 here
 - oxid-esales.com, per bug 428417 comment 0 (duplicate of this bug)
 - factcheck.org, per bug 429934 comment 4 (duplicate of this bug)
 - bugzilla.mozilla.org, per bug 429934 comment 5 (duplicate of this bug)

Comment 9

[Daniel Holbert [:dholbert]]

Note that the patch also moves the declaration of selectionHgt up a few lines, instead of recomputing that value twice, as we were before.

Comment 10

[Daniel Holbert [:dholbert]]

Also: fwiw, the patch *only* affects print-selection behavior -- the modified code is all within a "if (nsIPrintSettings::kRangeSelection == printRangeType)" clause.

Comment 11

[Daniel Holbert [:dholbert]]

Comment on attachment 316907 [details] [diff] [review]
fix v1

Requesting approval. Justification:
 - Low-risk
 - Scope is limited to print selection behavior (per comment 10)
 - Fixes print-selection issues on multiple sites (per comment 8)

Comment 12

[Daniel Holbert [:dholbert]]

FWIW, this comment in the patch
 +                // XXXdholbert does this num-pages calculation need to take
 +                // aPO->mZoomRatio into consideration?
can't be addressed until bug 428339 ("Print Selection output is limited to one page in trunk") is fixed.

(Once this patch lands, I'll mention this added comment on that bug's page.)

Comment 13

[Damon Sicore (:damons)]

Comment on attachment 316907 [details] [diff] [review]
fix v1

a1.9+=damons

Comment 14

[Daniel Holbert [:dholbert]]

fix v1 landed.

Checking in nsPrintEngine.cpp;
/cvsroot/mozilla/layout/printing/nsPrintEngine.cpp,v  <--  nsPrintEngine.cpp
new revision: 1.180; previous revision: 1.179
done

Comment 15

[Daniel Holbert [:dholbert]]

Attachment: [-- ignore me -- posted wrong file on wrong bug :) ]

Here's a second testcase, just like the first, but simplified by removing the margin from the body.

With fix v1, PDF-printouts of this testcase both with and without print-selection line up exactly in their positioning of the first pages' content.  
(They don't quite line up for testcase 1 because it has the default body margin, and print-selection ignores this body margin.)

Comment 16

[Daniel Holbert [:dholbert]]

Comment on attachment 317147 [details]
[-- ignore me -- posted wrong file on wrong bug :) ]

Oops... Ignore my last comment.  I meant to post a different testcase, and to post it on bug 430357, not here. :)

Comment 17

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=5288 added to Litmus.