Closed Bug 433373 Opened 16 years ago Closed 16 years ago

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

Categories

(Core :: Printing: Output, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9.1a1

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

(5 keywords)

Attachments

(7 files, 7 obsolete files)

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
Attached file 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.
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
Keywords: testcase
Attached patch patch v1 (obsolete) — Splinter Review
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: nobody → dholbert
Status: NEW → ASSIGNED
Attached patch patch v1a (added comments) (obsolete) — Splinter Review
Just added an explanatory comment to the patch.
Attachment #320591 - Attachment is obsolete: true
Attachment #320604 - Flags: superreview?(roc)
Attachment #320604 - Flags: review?(roc)
Requesting wanted-1.9.0.x, as this is a significantly broken print-selection regression with respect to FF2.
(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.
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
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
(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?
Target Milestone: --- → mozilla1.9
Attachment #320604 - Attachment description: patchy v1a (added comments) → patch v1a (added comments)
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.
Submitted testcase for RC 2 showing approximately 50% data loss with print selection.
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+
> 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?
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.
(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
Whiteboard: [has patch]
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+
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
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.
I am now forced to use Internet Explorer 7 to print since Firefox 3 RC 2 frequently results in data loss.
(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.)
(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/
> 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! 
Blocks: 439359
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+
Checked in to mozilla-central.
Changeset 5bffc31ff797
http://hg.mozilla.org/mozilla-central/index.cgi/rev/5bffc31ff797
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
Closed: 16 years ago
Resolution: --- → FIXED
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]
Attached patch patch v2 (uses page scaling) (obsolete) — Splinter Review
Here's the updated patch.
Attachment #326523 - Flags: superreview?(roc)
Attachment #326523 - Flags: review?(roc)
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)
Attachment #326524 - Attachment description: patch v2 (uses page scaling, based in mozilla/ directory) → patch v2a (uses page scaling, based in mozilla/ directory)
Attachment #326525 - Flags: superreview?(roc)
Attachment #326525 - Flags: review?(roc)
(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+
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+
Patch v2a checked in to mozilla-central:
 Changeset 354aed1b6240
 http://hg.mozilla.org/mozilla-central/index.cgi/rev/354aed1b6240
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
Closed: 16 years ago16 years ago
Keywords: fixed1.9.0.1
Resolution: --- → FIXED
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. 
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")
(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)
Alright thank you for the quick reply - looking forward to your fix making it to a release build.
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
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.
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
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.
Attached file Smaller text now with Firefox 3.0.1 (obsolete) —
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.
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.
(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)
(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")
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?
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.
Attached file Firefox 2 print selection behaviour (obsolete) —
This is how Firefox 2 always behaves (specific case here is today's firefox-2.0.0.17pre.en-US.win32 build).
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! 
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.
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.