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

VERIFIED FIXED in mozilla1.9.1a1

Status

()

Core
Printing: Output
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
mozilla1.9.1a1
dataloss, regression, relnote, testcase, verified1.9.0.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.0.1 -
wanted1.9.0.x +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments, 7 obsolete attachments)

2.08 KB, text/html
Details
13.57 KB, application/pdf
Details
35.73 KB, application/pdf
Details
21.41 KB, text/html
Details
1.74 KB, patch
Details | Diff | Splinter Review
848 bytes, patch
roc
: review+
Details | Diff | Splinter Review
26.84 KB, application/pdf
Details
(Assignee)

Description

9 years ago
Created attachment 320575 [details]
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.
(Assignee)

Comment 1

9 years ago
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.
Blocks: 431588
(Assignee)

Comment 2

9 years ago
Created attachment 320588 [details]
PDF-printed output (shows bug)
(Assignee)

Updated

9 years ago
Keywords: testcase
(Assignee)

Comment 3

9 years ago
Created attachment 320591 [details] [diff] [review]
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).
(Assignee)

Updated

9 years ago
Assignee: nobody → dholbert
(Assignee)

Updated

9 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 4

9 years ago
Created attachment 320604 [details] [diff] [review]
patch v1a (added comments)

Just added an explanatory comment to the patch.
Attachment #320591 - Attachment is obsolete: true
Attachment #320604 - Flags: superreview?(roc)
Attachment #320604 - Flags: review?(roc)
(Assignee)

Comment 5

9 years ago
Requesting wanted-1.9.0.x, as this is a significantly broken print-selection regression with respect to FF2.
(Assignee)

Comment 6

9 years ago
(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.
(Assignee)

Updated

9 years ago
Summary: Print-selection clips 1st page too soon & positions later pages too high, when I start selection lower down → Print-selection clips 1st page too soon & positions later pages too high, unless selection starts at very top
(Assignee)

Updated

9 years ago
Duplicate of this bug: 434864
(Assignee)

Comment 8

9 years ago
Adding 'dataloss' keyword, as this bug causes chunks of printed content to be shifted off of the page.
Flags: wanted1.9.0.x?
Keywords: dataloss
(Assignee)

Comment 9

9 years ago
(In reply to comment #6)
Changing 'wanted' request to blocking1.9.0.1, now that that flag's available.
Flags: wanted1.9.0.x? → blocking1.9.0.1?
(Assignee)

Updated

9 years ago
Target Milestone: --- → mozilla1.9
(Assignee)

Updated

9 years ago
Attachment #320604 - Attachment description: patchy v1a (added comments) → patch v1a (added comments)

Comment 10

9 years ago
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

9 years ago
Created attachment 323871 [details]
RC 2 (unpatched) still has 50% data loss for print selection

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

Comment 12

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

I'm guessing this fix will land in 3.0.1.
Attachment #320604 - Flags: superreview?(roc)
Attachment #320604 - Flags: superreview+
Attachment #320604 - Flags: review?(roc)
Attachment #320604 - Flags: review+

Comment 13

9 years ago
> 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

9 years ago
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.
(Assignee)

Comment 15

9 years ago
(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.
Keywords: relnote
(Assignee)

Updated

9 years ago
Whiteboard: [has patch]
(Assignee)

Comment 16

9 years ago
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)
--
Yep.  blocking1.9.0.1+.
Flags: blocking1.9.0.1? → blocking1.9.0.1+
(Assignee)

Updated

9 years ago
Attachment #323871 - Attachment description: RC 2 still has 50% data loss for print selection → RC 2 (unpatched) still has 50% data loss for print selection

Comment 18

9 years ago
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

9 years ago
I am now forced to use Internet Explorer 7 to print since Firefox 3 RC 2 frequently results in data loss.
(Assignee)

Comment 20

9 years ago
(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.)
(Assignee)

Comment 21

9 years ago
(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

9 years ago
> 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! 
(relnote added)
(Assignee)

Updated

9 years ago
Blocks: 439359
(Assignee)

Comment 24

9 years ago
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.
Attachment #320604 - Flags: approval1.9.0.1?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.1-
Flags: blocking1.9.0.1+
Comment on attachment 320604 [details] [diff] [review]
patch v1a (added comments)

a=beltzner, please watch for regressions
Attachment #320604 - Flags: approval1.9.0.1? → approval1.9.0.1+
(Assignee)

Comment 26

9 years ago
Checked in to mozilla-central.
Changeset 5bffc31ff797
http://hg.mozilla.org/mozilla-central/index.cgi/rev/5bffc31ff797
(Assignee)

Comment 27

9 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Comment 28

9 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [has patch]
(Assignee)

Comment 29

9 years ago
Created attachment 326521 [details]
testcase 2 (wide text; triggers page-scaling)
(Assignee)

Comment 30

9 years ago
Created attachment 326523 [details] [diff] [review]
patch v2 (uses page scaling)

Here's the updated patch.
Attachment #326523 - Flags: superreview?(roc)
Attachment #326523 - Flags: review?(roc)
(Assignee)

Comment 31

9 years ago
Created attachment 326524 [details] [diff] [review]
patch v2a (uses page scaling, based in mozilla/ directory)

Here's the fixed patch again, based in the mozilla/ directory to make interdiff happy.
Attachment #320604 - Attachment is obsolete: true
Attachment #326523 - Attachment is obsolete: true
Attachment #326523 - Flags: superreview?(roc)
Attachment #326523 - Flags: review?(roc)
(Assignee)

Updated

9 years ago
Attachment #326524 - Attachment description: patch v2 (uses page scaling, based in mozilla/ directory) → patch v2a (uses page scaling, based in mozilla/ directory)
(Assignee)

Comment 32

9 years ago
Created attachment 326525 [details] [diff] [review]
interdiff between patch v1a-2a
Attachment #326525 - Flags: superreview?(roc)
Attachment #326525 - Flags: review?(roc)
(Assignee)

Comment 33

9 years ago
(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
Attachment #326525 - Flags: superreview?(roc)
Attachment #326525 - Flags: superreview+
Attachment #326525 - Flags: review?(roc)
Attachment #326525 - Flags: review+
(Assignee)

Comment 34

9 years ago
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)
Attachment #326525 - Flags: approval1.9.0.1?
Comment on attachment 326525 [details] [diff] [review]
interdiff between patch v1a-2a

heck-yeah=beltzner
Attachment #326525 - Flags: approval1.9.0.1? → approval1.9.0.1+
Comment on attachment 326525 [details] [diff] [review]
interdiff between patch v1a-2a

a=shaver
(Assignee)

Comment 37

9 years ago
Patch v2a checked in to mozilla-central:
 Changeset 354aed1b6240
 http://hg.mozilla.org/mozilla-central/index.cgi/rev/354aed1b6240
(Assignee)

Comment 38

9 years ago
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
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Keywords: fixed1.9.0.1
Resolution: --- → FIXED

Comment 39

9 years ago
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. 
Keywords: fixed1.9.0.1 → verified1.9.0.1

Comment 40

9 years ago
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")
(Assignee)

Comment 41

9 years ago
(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

9 years ago
Alright thank you for the quick reply - looking forward to your fix making it to a release build.
(Assignee)

Comment 43

9 years ago
No prob -- thanks to you and Hasham for testing the nightly & verifying that it's fixed on your respective OS's.
Target Milestone: mozilla1.9 → mozilla1.9.1a1
Created attachment 328731 [details]
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.
(Assignee)

Comment 45

9 years ago
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.
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
Status: RESOLVED → VERIFIED

Comment 47

9 years ago
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

9 years ago
Created attachment 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/). 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

9 years ago
Created attachment 329945 [details]
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.
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.
(Assignee)

Comment 51

9 years ago
(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)
(Assignee)

Comment 52

9 years ago
(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

9 years ago
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

9 years ago
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

9 years ago
Created attachment 330026 [details]
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

9 years ago
Created attachment 330027 [details]
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.
George, as already mentioned in comment 51 please file a new bug on that issue! 
(Assignee)

Comment 58

9 years ago
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

9 years ago
Daniel, I've opened Bug 445894.
Attachment #329944 - Attachment is obsolete: true
Attachment #329945 - Attachment is obsolete: true
Attachment #330026 - Attachment is obsolete: true
Attachment #330027 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.