1668492 - Print paper size and scaling prefs are truncated in locales using a decimal separator other than "."

Status: RESOLVED FIXED

(Core :: Printing: Setup, defect, P2)

Description

[Jonathan Watt [:jwatt]]

[Spinning this off from bug 1668002 comment 13.]

The code for saving print settings as float prefs is locale aware, but the code for reading those same prefs is not:

https://searchfox.org/mozilla-central/rev/f21850ca45036ddb84cd25aa355a6969d7c94c4f/widget/nsPrintSettingsService.cpp#1007-1027

Specifically, nsPrintfCString is locale aware by virtue of it using nsTSubstring::AppendPrintf which is locale aware. However, atof is not locale aware and will stop parsing when it encounters a decimal separator that is not ".". In practice this means that for affected users pref values essentially undergo a floor() conversion on being read. So:

print_paper_height: 11,69 -> 11
print_paper_width: 8,27 -> 8
print_scaling: 1,50 -> 1

You'd think after 20 years we'd have noticed this bug, but I guess not. Other than the fact that the printing code has been unowned and 1000+ bug reports remain open, I guess this is because most users with a locale that uses a decimal separator other than "." probably use paper sizes in millimeters rather than inches, and paper sizes in millimeters don't tend to have a decimal component. Also I guess maybe this bug only causes some misscaling (resolution?) issues in the common case?

Comment 1

[Bob Owen (:bobowen)]

Just a note that this issue might have been made worse for the new UI.
At [1], we currently always set kPaperSizeInches.
On Windows this should get overridden by [2].

[1] https://searchfox.org/mozilla-central/source/widget/nsPrintSettingsImpl.cpp#75
[2] https://searchfox.org/mozilla-central/source/widget/windows/nsPrintSettingsWin.cpp#203

Comment 2

[Bob Owen (:bobowen)]

Attachment: Bug 1668492: Stop using locale based decimals when saving printing prefs. r=jwatt!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a38f6cd44b90
Stop using locale based decimals when saving printing prefs. r=jwatt

Comment 4

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a38f6cd44b90

Comment 5

[Julien Wajsberg [:julienw]]

Is that possible that after this landed, I now get a completely-disabled form when trying to print?

• Everything is disabled, with no indication why.
• I had the curiosity to click on "more parameters", and now I see that I have invalid values for the margins (they're all negative, see attachment).

When I move back the margin value to "default" this works again.

So I think there are 2 issues:

1. Maybe the float values I had before (see my about:support export in bug 1668002) were tried to be read as float and this is what gave this obviously wrong values?
2. Everything is disabled without any information, for the user this is very confusing.

Comment 6

[Bob Owen (:bobowen)]

(In reply to Julien Wajsberg [:julienw] from comment #5)

Is that possible that after this landed, I now get a completely-disabled form when trying to print?

• Everything is disabled, with no indication why.
• I had the curiosity to click on "more parameters", and now I see that I have invalid values for the margins (they're all negative, see attachment).

I think you might have missed the attachment.

When I move back the margin value to "default" this works again.

So I think there are 2 issues:

1. Maybe the float values I had before (see my about:support export in bug 1668002) were tried to be read as float and this is what gave this obviously wrong values?

I guess this is possible, but I'd be surprised. The patch for this bug should only have affected the reading/writing of paper height, width and scaling. Other changes might have landed that affect margins.
Would you be able to try and use mozregression to track down when this was introduced?

2. Everything is disabled without any information, for the user this is very confusing.

Yeah, that doesn't sound good, can you file this as a separate bug please.
If you can find steps to reproduce (including setting invalid prefs) that would be great.

Comment 7

[Julien Wajsberg [:julienw]]

Attachment: screenshot (french)

Comment 8

[Julien Wajsberg [:julienw]]

(In reply to Bob Owen (:bobowen) from comment #6)

2. Everything is disabled without any information, for the user this is very confusing.

Yeah, that doesn't sound good, can you file this as a separate bug please.
If you can find steps to reproduce (including setting invalid prefs) that would be great.

I filed bug 1669207.

Comment 9

[Julien Wajsberg [:julienw]]

(In reply to Bob Owen (:bobowen) from comment #6)

1. Maybe the float values I had before (see my about:support export in bug 1668002) were tried to be read as float and this is what gave this obviously wrong values?

I guess this is possible, but I'd be surprised. The patch for this bug should only have affected the reading/writing of paper height, width and scaling. Other changes might have landed that affect margins.
Would you be able to try and use mozregression to track down when this was introduced?

Right, mozregression gave me this pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=1a6c4e6d3320f2e3a7dbd2a3c1a23b624f7d8ccb&tochange=5f50537b7090c188ef15ca31ade7c93b1ddf04d1
which points to bug 1663503.