433373 - Print-selection clips 1st page too soon & positions later pages too high, unless selection starts at very top

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Steps to reproduce:
 1. Load testcase
 2. Select from line 20 to the end
 3. Print selection

Actual output: See upcoming PDF.  (First page's contents end abruptly, too soon, and subsequent pages are shifted up too high on their page.)

This a regression with respect to Firefox 2.

Comment 1

[Daniel Holbert [:dholbert]]

I think this is because the clipping region I set up in bug 430389 shouldn't necessarily start at 0 -- maybe it should start at mYSelOffset, off of the pageSequenceFrame.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: PDF-printed output (shows bug)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: patch v1

This fixes it.  This works because of how nsSimplePageFrame "slides" the nsPageContentFrame backwards as we go from page to page.

(Specifically, the single long page content frame gets a negative y-position such that the top of the current page is at position = 0 (discounting margins).

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: patch v1a (added comments)

Just added an explanatory comment to the patch.

Comment 5

[Daniel Holbert [:dholbert]]

Requesting wanted-1.9.0.x, as this is a significantly broken print-selection regression with respect to FF2.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to comment #5)
> Requesting wanted-1.9.0.x, as this is a significantly broken print-selection
> regression with respect to FF2.
... with an easy & safe fix.

Comment 8

[Daniel Holbert [:dholbert]]

Adding 'dataloss' keyword, as this bug causes chunks of printed content to be shifted off of the page.

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to comment #6)
Changing 'wanted' request to blocking1.9.0.1, now that that flag's available.

Comment 10

[George]

This is still not fixed in RC 2 (although it's a little bit better than RC 1).

The testcase that I submitted for bug 434864 is still broken:

1. Go to Firefox 3.0 RC 2 Release Notes page.
2. On the right, select the title "Firefox 3 (Release Candidate 2) Release Notes" and everything that is below it. (i.e. ignore the graphic on the top and the links on the left)
3. Ctrl+P and under "Print range" choose Selection.
4. The bottom half of every page is completely blank.

Actual Results:  
Data loss at the end of each page.

Comment 11

[George]

Attachment: RC 2 (unpatched) still has 50% data loss for print selection

Submitted testcase for RC 2 showing approximately 50% data loss with print selection.

Comment 12

[Daniel Holbert [:dholbert]]

Right -- there's a patch, but it's awaiting review.

I'm guessing this fix will land in 3.0.1.

Comment 13

[mojohdk]

> I'm guessing this fix will land in 3.0.1.

Argh - that's disappointing, Daniel.
Is there any way we can make some noise (perhaps by voting for the bug - it currently seems to have zero votes but Bug 402264 has 5....) so your patch makes it to the final release? This is the only bug i've noticed myself and it certainly         will annoy anyone trying to minimize their paper consumption. And it's a verifiable regression - surely nobody wants FF3 to look worse than FF2 on any account?

Comment 14

[George]

I agree with mojohdk. The test case I submitted shows 50% data loss. This is unacceptable.

If this regression is not fixed in time for 3.0, it should be clearly stated in the Release Notes that print selection can result in significant data loss.

Comment 15

[Daniel Holbert [:dholbert]]

(In reply to comment #13)
> Is there any way we can make some noise (perhaps by voting for the bug - it
> currently seems to have zero votes but Bug 402264 has 5....) so your patch
> makes it to the final release?

We pretty much can't -- Firefox 3 is close enough to final release (and awesome enough overall) that only *really* serious bugs would stop us from shipping at this point.  Any extra patches that we might take right now would mean extra QA that we have to do on those combined patches, and extra time we have to wait before shipping.

So, the alternative is to hold the patch for Firefox 3.0.1, which should be a pretty quick follow-up to 3.0.  I think that'll be fine in the case of this bug.  (particularly given that Print Selection was completely broken in the nightlies & betas before the bugs listed on 431588 were fixed, and relatively few people seemed to notice & file bugs. This bug's issue is a lot less severe than those ones. :))

> it should be clearly stated in
> the Release Notes that print selection can result in significant data loss.

Adding relnote keyword.

Comment 16

[Daniel Holbert [:dholbert]]

Suggested release note:
--
Some "Print Selection" output may be missing if the selected region starts in the middle of a document.  This has a fix and will soon be corrected in a follow-up release. (bug 433373)
--

Comment 17

[Damon Sicore (:damons)]

Yep.  blocking1.9.0.1+.

Comment 18

[George]

Daniel, I think your suggested release note isn't strong enough. As I've documented in bug 438326, even regular printing without print selection can result in Firefox 3.0 RC 2 "forgetting" to print the contents of the last page.

Comment 19

[George]

I am now forced to use Internet Explorer 7 to print since Firefox 3 RC 2 frequently results in data loss.

Comment 20

[Daniel Holbert [:dholbert]]

(In reply to comment #18)
> Daniel, I think your suggested release note isn't strong enough.

Suggested modifications are welcome.

> As I've
> documented in bug 438326, even regular printing without print selection can
> result in Firefox 3.0 RC 2 "forgetting" to print the contents of the last page.

FWIW, that bug has always been present in Mozilla/Firefox code. (it's a dupe of bug 129941, which was reported in 2002), and AFAIK it happens on a pretty small number of sites.

(In reply to comment #19)
> I am now forced to use Internet Explorer 7 to print since Firefox 3 RC 2
> frequently results in data loss.

Here's another option: right around when Firefox 3 is released, I'll check in this bug's fix, and then you can download the fixed nightly build from ( http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/ ) and try that out.  This bug should be fixed in that build, and you can then be helpful and verify that it does indeed fix it on your machine.

Or, you could wait a few weeks after Firefox 3 is released (which is how long we'd need to hold the release if we took additional patches right now, anyway, btw), and just grab Firefox 3.0.1 when it's available.

(And as for the normal-printing bug, bug 438326 -- if it hasn't stopped you from you from using previous versions of Firefox, I don't see why it should make you switch away now. :)  In all seriousness, I'd love to see that bug fixed as well, but it's not as high-priority as, say, this one, because it's not a regression.  It's also likely to be too destabilizing a change to take towards the end of a release cycle.  We'll probably nab that bug in the next Firefox release.)

Comment 21

[Daniel Holbert [:dholbert]]

(In reply to comment #20)
> (And as for the normal-printing bug, bug 438326 -- if it hasn't stopped you
> from you from using previous versions of Firefox,

er, s/you from you from/you from/

Comment 22

[mojohdk]

> I am now forced to use Internet Explorer 7 

I am actually using mostly the Mozilla Suite since their pre-1.0 days. I used to need to resort to IE to print back then, as I often select content from a page skipping graphics, printing in booklet format to save paper etc... which Mozilla couldn't handle at the time. 

I'll stick with SeaMonkey for now - no record download day for me; i'll eagerly await your fix. If you're referring to the one that was ready on 2008-05-12, it's too bad nobody allowed it to be incorporated in RC2. I thought the OSS motto was to release when it's ready, not when some marketing folk shouts "now!". 

Daniel (and all developers) thanks for listening to our gripes and taking care. Enjoy the (virtual) release party! 

Comment 23

[Mike Beltzner [:beltzner, not reading bugmail]]

(relnote added)

Comment 24

[Daniel Holbert [:dholbert]]

Comment on attachment 320604 [details] [diff] [review]
patch v1a (added comments)

Requesting approval1.9.0.1 (see justifications in comment 6 and comment 8)

Sadly, we don't have a way to make automated tests for print-selection yet (that's bug 428037), so this patch doesn't include tests.

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 320604 [details] [diff] [review]
patch v1a (added comments)

a=beltzner, please watch for regressions

Comment 26

[Daniel Holbert [:dholbert]]

Checked in to mozilla-central.
Changeset 5bffc31ff797
http://hg.mozilla.org/mozilla-central/index.cgi/rev/5bffc31ff797

Comment 27

[Daniel Holbert [:dholbert]]

Checked in to 1.9.0.x CVS branch.
Checking in nsPageFrame.cpp;
/cvsroot/mozilla/layout/generic/nsPageFrame.cpp,v  <--  nsPageFrame.cpp
new revision: 3.182; previous revision: 3.181
done

Comment 28

[Daniel Holbert [:dholbert]]

D'oh -- in foolproof-testing this a bit more, I realized I forgot to take page-scaliing into consideration.

I'll post a second testcase that demonstrates this other situation, with an updated patch, and I'll back out the broken patch shortly.

Comment 29

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (wide text; triggers page-scaling)

Comment 30

[Daniel Holbert [:dholbert]]

Attachment: patch v2 (uses page scaling)

Here's the updated patch.

Comment 31

[Daniel Holbert [:dholbert]]

Attachment: patch v2a (uses page scaling, based in mozilla/ directory)

Here's the fixed patch again, based in the mozilla/ directory to make interdiff happy.

Comment 32

[Daniel Holbert [:dholbert]]

Attachment: interdiff between patch v1a-2a

Comment 33

[Daniel Holbert [:dholbert]]

(In reply to comment #26)
> Checked in to mozilla-central.
> Changeset 5bffc31ff797
> http://hg.mozilla.org/mozilla-central/index.cgi/rev/5bffc31ff797

Backed out.  (changeset 294bf662e8c0)

(In reply to comment #27)
> Checked in to 1.9.0.x CVS branch.
> Checking in nsPageFrame.cpp;
> /cvsroot/mozilla/layout/generic/nsPageFrame.cpp,v  <--  nsPageFrame.cpp
> new revision: 3.182; previous revision: 3.181
> done

Backed out.

Checking in nsPageFrame.cpp;
/cvsroot/mozilla/layout/generic/nsPageFrame.cpp,v  <--  nsPageFrame.cpp
new revision: 3.183; previous revision: 3.182
done

Comment 34

[Daniel Holbert [:dholbert]]

Comment on attachment 326525 [details] [diff] [review]
interdiff between patch v1a-2a

Requesting approval on update to patch.

(see comment 24 & comment 25 for previous patch's approval request + granting)

Comment 35

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 326525 [details] [diff] [review]
interdiff between patch v1a-2a

heck-yeah=beltzner

Comment 36

[Mike Shaver (:shaver emeritus)]

Comment on attachment 326525 [details] [diff] [review]
interdiff between patch v1a-2a

a=shaver

Comment 37

[Daniel Holbert [:dholbert]]

Patch v2a checked in to mozilla-central:
 Changeset 354aed1b6240
 http://hg.mozilla.org/mozilla-central/index.cgi/rev/354aed1b6240

Comment 38

[Daniel Holbert [:dholbert]]

Patch v2a checked in to CVS (1.9.0.x branch)

Checking in nsPageFrame.cpp;
/cvsroot/mozilla/layout/generic/nsPageFrame.cpp,v  <--  nsPageFrame.cpp
new revision: 3.184; previous revision: 3.183
done

Comment 39

[Hasham]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.1) Gecko/2008070206 Firefox/3.0.1

Verified on 1.9.0.1. Prints as expected. 

Comment 40

[mojohdk]

Works on XP with this nightly, too. Thanks Daniel.

-- Mozilla/5.0 (Windows; U; Windows NT 5.1; en; rv:1.9.1a1pre) Gecko/2008070603 Minefield/3.1a1pre --

PS: is there a 3.0.1 with this problem fixed for windows, too? The update mechanism (from FF3-release) doesn't think so, apparently.
Also could someone point me to instructions on getting extensions installed on nightlies (e.g "web developer")

Comment 41

[Daniel Holbert [:dholbert]]

(In reply to comment #40)
> PS: is there a 3.0.1 with this problem fixed for windows, too?

3.0.1 hasn't been released yet -- it's in the QA process right now.  It should be out soon.  (presumably http://getfirefox.com/ will be updated with the new version when it's ready)

> Also could someone point me to instructions on getting extensions installed on
> nightlies (e.g "web developer")

Google for the "nightly tester tools" extension -- that's what you need. (not the "lite" version)

Comment 42

[mojohdk]

Alright thank you for the quick reply - looking forward to your fix making it to a release build.

Comment 43

[Daniel Holbert [:dholbert]]

No prob -- thanks to you and Hasham for testing the nightly & verifying that it's fixed on your respective OS's.

Comment 44

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Printed selection with fixed version

Daniel, could you please have a look at this printed pdf? As you can see there is still a garbled line at the end of the first page. Does it depend on this bug or is it another one?

I updated the Target Milestone because the version is marked as trunk and 1.9.0.x is verified within the whiteboard.

Comment 45

[Daniel Holbert [:dholbert]]

Henrik - thanks for the question -- that last line in your attachment isn't garbled, it's merely split across two pages.  (The first page has the very top of the line, and the second page has the rest)

That's just a product of the way print-selection works in the Mozilla codebase -- we render the selection to one long printed page, and then we snip that into page-sized chunks.  This strategy does cause occasional line-splitting, but AFAIK that's unavoidable without a major reworking of print-selection.

Comment 46

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, I'll have a look if I can find the bug which handles this issue.

I can verify this bug with

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a1pre) Gecko/2008070902 Minefield/3.1a1pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a1pre) Gecko/2008070903 Minefield/3.1a1pre ID:2008070903

Comment 47

[George]

Daniel, this problem seems to fixed on Firefox 3.0.1 that was released today. (I tested on Windows XP SP3.) Thanks for fixing this problem. However, I am seeing that text is now noticeably smaller than before. Is this related to the fix for this problem or another issue? I will include a test case now.

Comment 48

[George]

Attachment: Smaller text now with Firefox 3.0.1

Here is what Firefox 3.0.1 prints when I print a selection of the "Firefox Features" web page (http://www.mozilla.com/en-US/firefox/features/). I selected from the "Firefox Features" title down to the end of "Firefox Support".

Internet Explorer 7 test case will now follow to illustrate the difference.

Comment 49

[George]

Attachment: Internet Explorer 7 print selection (compare with my previous attachment)

Internet Explorer 7 print selection of exactly the same material as shown in my previous attachment. Text is the expected size.

Comment 50

[Ryan VanderMeulen [:RyanVM]]

George, please file a new bug for the issue you're seeing. It makes it harder for everybody to work efficiently when multiple problems are being raised in one bug.

Comment 51

[Daniel Holbert [:dholbert]]

(In reply to comment #48)
> Created an attachment (id=329944) [details]
> Smaller text now with Firefox 3.0.1
> 
> Here is what Firefox 3.0.1 prints when I print a selection of the "Firefox
> Features" web page (http://www.mozilla.com/en-US/firefox/features/).

The small text in that attachment is normal. That just happens when the content is too wide to fit on a page (e.g. due to a wide image, or a div with a large specified width), and so Firefox shrinks it to make it fit.  

I'm not sure what IE's doing differently, but IMHO it's not a huge deal that we give slightly  different print-selection output from them. :)

> However, I am
> seeing that text is now noticeably smaller than before.

If you *definitely* get smaller text in FF3.0.0 vs. 3.0.1, then please file a new bug, and attach printed PDFs from both versions.

I'm guessing you might be mistaken, because I tested your steps in Comment 48 with both FF3.0.0 and FF3.0.1, and they give the exact same  size text on my machine. (Ubuntu Linux 8.04)

Comment 52

[Daniel Holbert [:dholbert]]

(In reply to comment #51)
> If you *definitely* get smaller text in FF3.0.0 vs. 3.0.1
(oops, sorry -- make that "smaller text in FF3.0.1 vs 3.0.0")

Comment 53

[George]

I meant Firefox 3 (3.0 and 3.0.1) is noticeably smaller than Firefox 2 (e.g. 2.0.0.14). Is this the expected behaviour?

> The small text in that attachment is normal. That just happens
> when the content is too wide to fit on a page

As shown in the Firefox 3.0.1 attachment, there is too much white space for the left and right margins. There is nothing too wide that cannot fit.

I frequently print reports from my library (using print selection). With Firefox 2 and IE 7, the reports would print on two pages in easy-to-read text. Now with Firefox 3, everything is just squished down onto one page. Is this the expected new behaviour of Firefox 3?

Comment 54

[George]

To illustrate my point with Firefox 3 vs. Firefox 2, I have just installed firefox-2.0.0.17pre.en-US.win32 (today's build). I have done a print selection of exactly the same data. Firefox 3.0.1 shrinks everything to fit on the page, whereas Firefox 2 never did that (it behaved similarly to IE 7).

I will upload both PDF files now as attachments.

Comment 55

[George]

Attachment: Firefox 2 print selection behaviour

This is how Firefox 2 always behaves (specific case here is today's firefox-2.0.0.17pre.en-US.win32 build).

Comment 56

[George]

Attachment: Firefox 3.0.1 Print Selection (c.f. Firefox 2 behaviour)

Here is Firefox 3.0.1. Everything is squished down to fit on one page. This didn't happen with Firefox 2 or IE 7.

Comment 57

[Henrik Skupin [:whimboo][⌚️UTC+2]]

George, as already mentioned in comment 51 please file a new bug on that issue! 

Comment 58

[Daniel Holbert [:dholbert]]

George - Firefox 2 scales text down sometimes, too (try attachment 316894 [details], which gives me shrunk output on FF2 and FF3).  As mentioned in Comment 51, this is normal behavior, and it happens when content is too small to fit on a page. I'm not sure why FF3 and FF2 disagree about whether the page contents are too wide, in the examples you posted.

> As shown in the Firefox 3.0.1 attachment, there is too much white space for the
> left and right margins. There is nothing too wide that cannot fit.

I don't see that whitespace when I print-selection that page on my machine.  However, I've seen that happen in other cases before -- usually it's because Firefox saves some space for a sidebar on the original page, even though that sidebar isn't selected.  I'd have to see a minimized testcase to know why it's happening in this particular case.

So, I'm not too worried about this, but go ahead and file another bug if you want this looked into.

Comment 59

[George]

Daniel, I've opened Bug 445894.