Closed Bug 430878 Opened 15 years ago Closed 15 years ago

Print-selection doesn't print lower "overflow:scroll" divs unless their upper mirror-image is selected.

Categories

(Core :: Printing: Output, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

(Keywords: regression, testcase)

Attachments

(3 files)

In the testcase I'm about to attach, print-selection shows the following strange properties:

A. Selected text is *not* moved to the top of the page, as it should be. 
  (e.g. if I print-selection with just "3" selected, the "3" isn't moved to the top of the page -- it's instead on the 3rd line, at the same place it'd be on a full printout of the page.)

B. The lower lines (lines 5-7) are never printed in print-selection, *UNLESS* their upper mirror-image lines are also selected.
  e.g. line 5 only prints if I've selected lines 5 *and* 3.
  e.g. line 6  only prints if I've selected lines 6 *and* 2
  e.g. if I select lines 3-7, only lines 3-5 are printed. (lines 6 and 7 are missing unless I also select lines 2 and 1)

Both of these behaviors seem to be due to the "overflow" CSS property being set.

Note: You may need to hold the Ctrl key in order to select multiple lines on this testcase.

(This testcase was minimized from reddit.com -- currently, I can print-selection if I select a chunk of the first article title there, but not if I select a chunk from the second or later ones.  I think that's related to this bug.)
Assignee: nobody → dholbert
Attached file testcase 2
This testcase has fewer lines, and some plain (not overflow:hidden) text at the end.  This ending text makes it easier to select multiple lines in FF2 -- just start by selecting some of that ending text, and then drag the cursor upwards.

(multiline-selection doesn't seem possible on testcase 1 in FF2, AFAICT -- it looks like when you start selecting, it only selects within the current overflow-hidden div.)

Anyway -- testcase 2 shows that this is a regression, per this new testcase and the following experiments:

Experiment B:
 1. Click in between "b" and "c", and then move the cursor upwards, so that you've selected "3" and "ab".
 2. Print selection

RESULTS ON FF3: only "abc" is printed, shifted a few lines down.

Experiment C:
 1. Click in between "b" and "c", and then move the cursor upwards, so that you've selected "2", "3", and "ab".
 2. Print selection

RESULTS ON FF3: 2, 3, and "abc" are all printed, but the "3" and the "abc" overlap.  And there's a blank line at the top.

Results on FF2 are what you'd expect -- the selected portion prints, it's aligned with the top of the page, and nothing overlaps.
Ok, so it looks like the main thing going wrong here is that scrollframes' contents aren't being slid to the top of the document, with the rest of the content.

This causes the "upper-mirror-image needs to be selected" symptom in this bug, for this reason:  When we slide the page content frame upwards by mYSelOffset ( = y position of first selection), then the last unslid scrollframe that will still be in the bounds of the shifted page content frame will be the "mirror image" of the first selected item.

So, to fix this, we need to make sure that scrollframes get slid upwards by mYSelOffset, together with the rest of the page.
We need to reposition the scrollframes' views, after the page content frame is shifted.  PositionChildViews does just that. 

See this comment right above its implementation, at
http://mxr.mozilla.org/seamonkey/source/layout/generic/nsContainerFrame.cpp#791

791  * Position the views of |aFrame|'s descendants. A container frame
792  * should call this method if it moves a frame after |Reflow|.

So, we definitely need to be calling this function.

This fixes testcase 1, testcase 2, and reddit.com.  In the experiments described in Comment 0 and Comment 2, the selected text is now visible *and* is shifted correctly to the top of the page, whereas it wasn't before.
Attachment #318251 - Flags: superreview?(roc)
Attachment #318251 - Flags: review?(roc)
Attachment #318251 - Flags: superreview?(roc)
Attachment #318251 - Flags: superreview+
Attachment #318251 - Flags: review?(roc)
Attachment #318251 - Flags: review+
Comment on attachment 318251 [details] [diff] [review]
patch v1: call PositionChildViews after shifting page content frame

Requesting review.
 * Benefits
   - Fixes print-selection dataloss on major site (reddit)

 * Risk: Minimal
   - Very simple patch
   - only affects print-selection
   - Clearly the right thing to do, per the quoted code-comment in comment 4
Attachment #318251 - Flags: approval1.9?
(In reply to comment #5)
> Requesting review.

Sorry -- I meant "requesting approval" :)
Status: NEW → ASSIGNED
Comment on attachment 318251 [details] [diff] [review]
patch v1: call PositionChildViews after shifting page content frame

a1.9+=damons
Attachment #318251 - Flags: approval1.9? → approval1.9+
Patch v1 checked in.

Checking in layout/generic/nsSimplePageSequence.cpp;
/cvsroot/mozilla/layout/generic/nsSimplePageSequence.cpp,v  <--  nsSimplePageSequence.cpp
new revision: 3.172; previous revision: 3.171
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.