Shrink to fit not working reliably

RESOLVED FIXED in mozilla13

Status

()

Core
Printing: Output
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Ignaz Forster, Assigned: Michael Ventnor)

Tracking

(Depends on: 1 bug, {testcase})

unspecified
mozilla13
x86
Linux
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
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
martijn: do you have a idea who is best to get cc'd to the bug ?
Keywords: testcase
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? ;)
Sure!
Assignee: nobody → ventnor.bugzilla
(Assignee)

Comment 4

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

Comment 5

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

Comment 7

6 years ago
Found it, I was thinking of bug 394103, but I mis-remembered the fix we implemented for that.
(Assignee)

Comment 8

6 years ago
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.
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

6 years ago
(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.
Submit a patch?

Comment 12

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

Comment 14

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

6 years ago
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.
Just remove the lines. We don't want our source code cluttered up with commented-out lines :-).

Comment 17

6 years ago
Created attachment 592498 [details] [diff] [review]
shrink to fit fix (updated), patches against firefox nightly

updated fix
Attachment #592405 - Attachment is obsolete: true
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"
Attachment #592498 - Flags: review+
Attachment #592498 - Flags: checkin?

Comment 19

6 years ago
Created attachment 592613 [details] [diff] [review]
shrink to fit fix (updated), patches against firefox nightly

proper commit message
Attachment #592498 - Attachment is obsolete: true
Attachment #592498 - Flags: checkin?

Comment 20

6 years ago
Created attachment 592614 [details] [diff] [review]
shrink to fit fix (updated), patches against firefox nightly

wrong email
Attachment #592613 - Attachment is obsolete: true
Attachment #592614 - Flags: review+
Attachment #592614 - Flags: checkin?
(Curious to try out autoland, here goes... :-))
Whiteboard: [autoland-try]

Updated

6 years ago
Whiteboard: [autoland-try] → [autoland-in-queue]

Comment 22

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

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

Updated

6 years ago
Whiteboard: [autoland-in-queue]
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a35540e409b
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla13

Updated

6 years ago
Attachment #592614 - Flags: checkin? → checkin+
The clamping got implemented with bug 135013
https://hg.mozilla.org/mozilla-central/rev/7a35540e409b
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 768348

Updated

5 years ago
Depends on: 812095
You need to log in before you can comment on or make changes to this bug.