Last Comment Bug 680436 - Shrink to fit not working reliably
: Shrink to fit not working reliably
Status: RESOLVED FIXED
: testcase
Product: Core
Classification: Components
Component: Printing: Output (show other bugs)
: unspecified
: x86 Linux
: -- normal with 1 vote (vote)
: mozilla13
Assigned To: Michael Ventnor
:
:
Mentors:
Depends on: 812095 768348
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-19 07:54 PDT by Ignaz Forster
Modified: 2012-11-15 08:47 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Minimal testcase (395 bytes, text/html)
2011-08-19 07:54 PDT, Ignaz Forster
no flags Details
shrink to fit fix, patches against firefox nightly (628 bytes, patch)
2012-01-28 08:47 PST, Florian Maier
no flags Details | Diff | Splinter Review
shrink to fit fix (updated), patches against firefox nightly (552 bytes, patch)
2012-01-29 06:23 PST, Florian Maier
roc: review+
Details | Diff | Splinter Review
shrink to fit fix (updated), patches against firefox nightly (802 bytes, patch)
2012-01-29 21:56 PST, Florian Maier
no flags Details | Diff | Splinter Review
shrink to fit fix (updated), patches against firefox nightly (800 bytes, patch)
2012-01-29 22:01 PST, Florian Maier
roc: review+
emorley: checkin+
Details | Diff | Splinter Review

Description Ignaz Forster 2011-08-19 07:54:04 PDT
Created attachment 554416 [details]
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] 2011-08-19 07:58:28 PDT
martijn: do you have a idea who is best to get cc'd to the bug ?
Comment 2 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-08-25 06:05:23 PDT
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) 2011-08-25 22:31:42 PDT
Sure!
Comment 4 Michael Ventnor 2011-08-28 22:25:26 PDT
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 2011-08-29 22:17:24 PDT
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) 2011-08-29 22:18:17 PDT
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 2011-08-29 22:27:15 PDT
Found it, I was thinking of bug 394103, but I mis-remembered the fix we implemented for that.
Comment 8 Michael Ventnor 2011-08-30 22:14:50 PDT
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) 2011-08-30 22:19:37 PDT
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 2012-01-24 06:35:10 PST
(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) 2012-01-24 13:28:31 PST
Submit a patch?
Comment 12 Florian Maier 2012-01-25 11:55:23 PST
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) 2012-01-25 20:29:05 PST
Yes please
Comment 14 Florian Maier 2012-01-28 08:47:00 PST
Created attachment 592405 [details] [diff] [review]
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 2012-01-28 08:51:49 PST
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) 2012-01-28 19:52:02 PST
Just remove the lines. We don't want our source code cluttered up with commented-out lines :-).
Comment 17 Florian Maier 2012-01-29 06:23:54 PST
Created attachment 592498 [details] [diff] [review]
shrink to fit fix (updated), patches against firefox nightly

updated fix
Comment 18 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-01-29 11:37:54 PST
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 2012-01-29 21:56:27 PST
Created attachment 592613 [details] [diff] [review]
shrink to fit fix (updated), patches against firefox nightly

proper commit message
Comment 20 Florian Maier 2012-01-29 22:01:52 PST
Created attachment 592614 [details] [diff] [review]
shrink to fit fix (updated), patches against firefox nightly

wrong email
Comment 21 Ed Morley [:emorley] 2012-02-03 14:56:32 PST
(Curious to try out autoland, here goes... :-))
Comment 22 Mozilla RelEng Bot 2012-02-03 15:02:26 PST
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 2012-02-03 20:45:20 PST
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/autolanduser@mozilla.com-4dc2ce787aa5
Comment 25 Matthias Versen [:Matti] 2012-02-04 09:19:21 PST
The clamping got implemented with bug 135013
Comment 26 Ed Morley [:emorley] 2012-02-05 04:11:48 PST
https://hg.mozilla.org/mozilla-central/rev/7a35540e409b

Note You need to log in before you can comment on or make changes to this bug.