680436 - Shrink to fit not working reliably

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Ignaz Forster]

Attachment: Minimal testcase

Opening the attached minimal file in Firefox (tested versions 3.6, 4 and 6) and using the print preview, the site is cut off on the right even though "Shrink to fit" is selected. The same thing happens when printing directly and having selected scaling in the printer settings.

This behaviour only occurs on Linux (Ubuntu | Arch), using Windows the scaling seems to be done correctly.


Related bugs (where the linked testcases or pages also fail):
#356358
#130850
#207083
#212303

Comment 1

[Carsten Book [:Tomcat]]

martijn: do you have a idea who is best to get cc'd to the bug ?

Comment 2

[Martijn Wargers (dead)]

Indeed, I can't reproduce on Windows.
For linking purposes, related bugs: bug 356358, bug 130850, bug 207083, bug 212303.

I don't think there are real developers working on these kind of printing problems.
Perhaps Robert can order one of his slaves to work on it? ;)

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sure!

Comment 4

[Michael Ventnor]

It always has to be me... :)

So I'm really running out of ideas for why this is occurring.

- It has nothing to do with the "size" attribute, CSS widths exhibit the same behaviour.
- It has nothing to do with native widgets, -moz-appearance: none does the same thing.
- It has nothing to do with use of tables, as a wide div doesn't fix it.
- I can't find anything odd in nsTextControlFrame.

And yet it's only a problem on Linux??? This is very strange. I'll have to see if it's a prob on Mac.

Comment 5

[Michael Ventnor]

Still reading code, still not seeing anything odd.

If I replace everything with a wide div, the problem still persists.

There's only one thing I can possibly think of. roc, do you remember that bug regarding Linux's use of DPI? Didn't we fix that by always setting the DPI to a fixed value, or something like that, due to quirks in X? What bug was that?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't know what you mean.

You should probably just debug the code that computes the scale factor we use for shrink-to-fit.

Comment 7

[Michael Ventnor]

Found it, I was thinking of bug 394103, but I mis-remembered the fix we implemented for that.

Comment 8

[Michael Ventnor]

Aha, found it in nsPrintEngine.cpp:

      // Clamp Shrink to Fit to 60%
      mPrt->mShrinkRatio = NS_MAX(mPrt->mShrinkRatio, 0.60f);

This was written before the hg switch so I can't find the blame for it or the reason it was written.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/printing/nsPrintEngine.cpp&rev=1.184
https://bugzilla.mozilla.org/show_bug.cgi?id=175316

Not much info there...

Comment 10

[Florian Maier]

(In reply to Michael Ventnor from comment #8)
> Aha, found it in nsPrintEngine.cpp:
> 
>       // Clamp Shrink to Fit to 60%
>       mPrt->mShrinkRatio = NS_MAX(mPrt->mShrinkRatio, 0.60f);
> 
> This was written before the hg switch so I can't find the blame for it or
> the reason it was written.

I just checked with firefox 10 (b5). The bug is still there, removing this line in our internal builds fixes the problem for me. An upstream fix would be great.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Submit a patch?

Comment 12

[Florian Maier]

If anyone is really interested in this one line fix (removing the line), sure.
Should i submit a patch against nightly?

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes please

Comment 14

[Florian Maier]

Attachment: shrink to fit fix, patches against firefox nightly

patch against firefox nightly. This patch removes the 60% clamp for the shrink to fit feature.

Comment 15

[Florian Maier]

I also verified the fix to work for the OS X build of Firefox. At least OS X Lion (10.7.2) shows the same behaviour as Linux. With the patch applied, prints looked as desired/expected.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Just remove the lines. We don't want our source code cluttered up with commented-out lines :-).

Comment 17

[Florian Maier]

Attachment: shrink to fit fix (updated), patches against firefox nightly

updated fix

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 592498 [details] [diff] [review]
shrink to fit fix (updated), patches against firefox nightly

We'll need an appropriate commit message. Probably "Bug 680436. Don't clamp shrink-to-fit values. r=roc"

Comment 19

[Florian Maier]

Attachment: shrink to fit fix (updated), patches against firefox nightly

proper commit message

Comment 20

[Florian Maier]

Attachment: shrink to fit fix (updated), patches against firefox nightly

wrong email

Comment 21

[Ed Morley [:emorley]]

(Curious to try out autoland, here goes... :-))

Comment 22

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 592614
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/rev/4dc2ce787aa5
Try run started, revision 4dc2ce787aa5. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=4dc2ce787aa5

Comment 23

[Mozilla RelEng Bot]

Try run for 4dc2ce787aa5 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=4dc2ce787aa5
Results (out of 207 total builds):
    exception: 1
    success: 182
    warnings: 24
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-4dc2ce787aa5

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a35540e409b

Comment 25

[Matthias Versen [:Matti]]

The clamping got implemented with bug 135013

Comment 26

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7a35540e409b