223111 - Shrink to fit and Scaling values do not persist

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Jessica Blanco]

Every time you start the browser, shrink to fit is selected in the settings when
printing. You can't set to print at 100% once and have every printout be at 100%
because next time you start the browser shrink to fit will be selected again.
The browser should be able to save shrink to fit and scaling values as prefs
(like print orientation and margins) to be saved throughout all mozilla sessions.

Comment 1

[Jessica Blanco]

Attachment: patch to read/write shrink to fit and scaling values

I added the code to read in and write out the values for shrink to fit and
scaling in the prefs. I also added the flags to do this in page setup and
printing. I changed the page setup dialog to only allow integer values for
scaling because only 3 digits were being saved anyway. Updated
printPreviewBindings so the scaling combobox works correctly after custom
values are entered or the dialog has been cancelled.  Fixed
nsDeviceContextSpecOS2.cpp because the default printer name was never being
set.  I did change PR_sscanf() to atof() in this patch because the values
weren't coming back correctly with PR_sscaf(), but would always come back
correctly with atof().

Comment 2

[Johnny Stenback (:jst)]

Attachment: Same as Jessica's patch, but no more lastValidInx attribute, and leaving the scale factor as a float.

So this does pretty much what Jessica's patch did, but w/o making the scale
factor be an int, which I think was a mistake. We're using more than 3 digits
from it, we're using 3 or the given decimals, and with this patch we preserve
all that. I also eliminated the lastValidIdx from printPreviewBindings.xml
since I don't see a need for it, and this way we don't have any odd hard-coded
number in our code.

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 137277 [details] [diff] [review]
Same as Jessica's patch, but no more lastValidInx attribute, and leaving the scale factor as a float.

That wasn't quite right, better patch coming up.

Comment 4

[timeless]

Comment on attachment 137277 [details] [diff] [review]
Same as Jessica's patch, but no more lastValidInx attribute, and leaving the scale factor as a float.

I'm not very fond of this kind of change:
-                gPrintSetInterface.kInitSavePrintCommand;
+                gPrintSetInterface.kInitSavePrintCommand  |
...
+                gPrintSetInterface.kInitSaveScaling;

Are you opposed to:
+                gPrintSetInterface.kInitSaveScaling       |
+                0;

With the expectation that people would insert before 0.

Comment 5

[Johnny Stenback (:jst)]

Attachment: This should do it.

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 137286 [details] [diff] [review]
This should do it.

Ben says r=ben.

Comment 7

[Johnny Stenback (:jst)]

> I'm not very fond of this kind of change:
> -                gPrintSetInterface.kInitSavePrintCommand;
> +                gPrintSetInterface.kInitSavePrintCommand  |
> ...
> +                gPrintSetInterface.kInitSaveScaling;
> 
> Are you opposed to:
> +                gPrintSetInterface.kInitSaveScaling       |
> +                0;

I think my answer would be yes, I don't see the problem with the above? Is it
that the diff isn't a two line diff when it could be? If so, I really don't
care, I care more about the code you get when you apply the diff than what the
diff looks like. Whata you're suggesting actually has a runtime penalty (during
JS compilation if nothing else)...

Comment 8

[Mike Kaply [:mkaply]]

I'd really like this for 1.6. We get a lot of complaints about it.

Comment 9

[Asa Dotzler [:asa]]

Comment on attachment 137286 [details] [diff] [review]
This should do it.

a=asa (on behalf of drivers) for checkin to Mozilla 1.6

Comment 10

[Ed Beu]

Looks like great progress...will work for bug#147384 too. Will there be a
"nightly build" to test before 1.6 final? Kudos to you programmers!

Comment 11

[Asa Dotzler [:asa]]

mkaply, can you land this? I think jst is away on vacation and this won't make
1.6 without help landing. 

Comment 12

[Mike Kaply [:mkaply]]

I will land this tomorrow AM

Comment 13

[Johnny Stenback (:jst)]

I'm back on my feet (from Finland), and I just landed this. FIXED.

Comment 14

[Johnny Stenback (:jst)]

Ed, the nightly builds starting from the 17th should have this change. Please
report back if you have a chance to try one out!

Comment 15

[Ed Beu]

JTS, please see recent comments on bug#147384, just downloaded nightly build -
which didn't appear to be different from yesterdays...- same negative results.

Comment 16

[Peter Weilbacher]

Hmm, this is supposed to be resolved, but:

- I create a new profile and start Mozilla
- It goes to http://www.mozilla.org/start/
- I select "Print Preview" and a scale of 80%.
- I select "Close"
- I exit Mozilla and start it again with http://www.mozilla.org/start/ as argument
- I select "Print Preview" and see the scale set to "Shrink To Fit" again.

If I print to a file before I click "Close" the scale is indeed saved. Should I
open a new bug for this or is it intended the scaling is not saved when I decide
not to print the page at this time?

I tried the above with Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.6)
Gecko/20040110.

Comment 17

[Johnny Stenback (:jst)]

I guess that's a question of what one wants, IMO if you close the print preview
window after making changes you could see that as canceling the printing, and
you didn't want any of the changes you made. IOW, I think a new bug should be
filed, unless you agree with me on this issue and decide that what Mozilla now
does is "correct" :-)