1493955 - Floating-point conversions in preferences are not locale-independent

Status: RESOLVED FIXED

(Core :: Preferences: Backend, defect)

Description

[Gabriele Svelto [:gsvelto]]

I just run into an assertion failure because one of the static preferences was being set to a non-integral floating-point value (layout.accessiblecaret.margin-left) and converted back to the wrong value. The floating point string representing the value is "-18.50000" and on my system it gets parsed into "-18.00000" by the ToFloat() method because the decimal point in my locale is ',' instead of '.'.

Comment 1

[Gabriele Svelto [:gsvelto]]

Strangely enough this seems to be happening only with static preferences, other floating-point values are being treated correctly. I'll investigate further.

Comment 2

[Gabriele Svelto [:gsvelto]]

I've figured it out: static preferences are inserted into the hash-table using SetPref_*() calls and SetPref_float() uses an nsPrintfCString variable to convert the floating-point value to a string. Unfortunately nsPrintfCString is locale-dependent so on my machine it will use a comma as the decimal separator. When the variable is read the string is converted back into a float using nsTString.ToFloat() which relies on PR_strtod() which on the other hand is locale-independent and thus fails to parse the localized string.

I'm hacking together a patch to fix this.

Comment 3

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1493955 - Store floating-point preferences in a locale-independent way

Comment 4

[Nicholas Nethercote [inactive]]

Comment on attachment 9011844 [details]
Bug 1493955 - Store floating-point preferences in a locale-independent way

Nicholas Nethercote [:njn] has approved the revision.

Comment 6

[Gabriele Svelto [:gsvelto]]

Ouch, touching the unit-tests turned up something nasty: rounding seems to also be locale-dependent so the existing basic tests fail on my machine :-| I'll have to fix that too.

Comment 7

[Gabriele Svelto [:gsvelto]]

I've updated the patch, the try run is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a7486cf5c36d89ff0d35d5fe3ed73d6f66161cb

Comment 8

[Gabriele Svelto [:gsvelto]]

gtest didn't like comparisons between NULL and nullptr and the Mac build was broken :-/ Here's another try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b74bb2bfc9b6558d9b94e794d5f6b94c5adb508d

Comment 9

[Gabriele Svelto [:gsvelto]]

OK, our automation VMs don't seem to support locales that aren't en_US which I almost expected. I still want to have the unit-tests in, but I'll make them skip the test instead of failing if a non-en_US locale is not available so at least they can be run locally.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba1fef7b14eb
Store floating-point preferences in a locale-independent way r=njn

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset ba1fef7b14eb (Bug 1493955) for GTest failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ba1fef7b14eb4fd87ae72231196ddf29cd5fc5a6

Backout link: https://hg.mozilla.org/integration/autoland/rev/6cc26ea43938b62443c992908f0fbdd9d4333c29

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=202072776&repo=autoland&lineNumber=750

15:38:09     INFO -    --app=APP             Application being installed. [default: firefox]
15:38:09     INFO - mkdir: /Users/cltbld/tasks/task_1538087754/build/application
15:38:09     INFO - Getting output from command: ['/Users/cltbld/tasks/task_1538087754/build/venv/bin/mozinstall', '/Users/cltbld/tasks/task_1538087754/installer.dmg', '--destination', '/Users/cltbld/tasks/task_1538087754/build/application']
15:38:09     INFO - Copy/paste: /Users/cltbld/tasks/task_1538087754/build/venv/bin/mozinstall /Users/cltbld/tasks/task_1538087754/installer.dmg --destination /Users/cltbld/tasks/task_1538087754/build/application
15:38:32     INFO - Reading from file tmpfile_stdout
15:38:32     INFO - Output received:
15:38:32     INFO -  /Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/MacOS/firefox
15:38:32     INFO - Running post-action listener: _resource_record_post_action
15:38:32     INFO - [mozharness: 2018-09-27 22:38:32.293989Z] Finished install step (success)
15:38:32     INFO - [mozharness: 2018-09-27 22:38:32.294125Z] Running stage-files step.
15:38:32     INFO - Running pre-action listener: _resource_record_pre_action
15:38:32     INFO - Running main action method: stage_files
15:38:32     INFO - Moving /Users/cltbld/tasks/task_1538087754/build/tests/bin/plugins/gmp-clearkey to /Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/Resources
15:38:32    ERROR - shutil error: Destination path '/Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/Resources/gmp-clearkey' already exists
15:38:32     INFO - Moving /Users/cltbld/tasks/task_1538087754/build/tests/bin/plugins/gmp-fake to /Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/Resources
15:38:32     INFO - Moving /Users/cltbld/tasks/task_1538087754/build/tests/bin/plugins/gmp-fakeopenh264 to /Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/Resources
15:38:32     INFO - Moving /Users/cltbld/tasks/task_1538087754/build/tests/gtest/dependentlibs.list.gtest to /Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/Resources
15:38:32     INFO - copying tree: /Users/cltbld/tasks/task_1538087754/build/tests/gtest/gtest_bin to /Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/MacOS
15:38:32     INFO - mkdir: /Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/MacOS/gtest
15:38:32     INFO - copying tree: /Users/cltbld/tasks/task_1538087754/build/tests/gtest/gtest_bin/gtest to /Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/MacOS/gtest
15:38:32     INFO - rmtree: /Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/MacOS/gtest
15:38:32     INFO - retry: Calling rmtree with args: ('/Users/cltbld/tasks/task_1538087754/build/application/Firefox Nightly.app/Contents/MacOS/gtest',), kwargs: {}, attempt #1
15:38:32     INFO - Running post-action listener: _resource_record_post_action

Comment 12

[Gabriele Svelto [:gsvelto]]

The error in the failed run seems to be a already known as an intermittent failure, bug 1370165. I'll re-run on try and re-land but I don't think anything in this change might be responsible for that failure.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/326be6e41703
Store floating-point preferences in a locale-independent way r=njn

Comment 14

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/326be6e41703