Closed Bug 1493955 Opened 6 years ago Closed 6 years ago

Floating-point conversions in preferences are not locale-independent

Categories

(Core :: Preferences: Backend, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- unaffected
firefox64 --- fixed

People

(Reporter: gsvelto, Assigned: gsvelto)

References

Details

(Keywords: regression)

Attachments

(1 file)

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 '.'.
Strangely enough this seems to be happening only with static preferences, other floating-point values are being treated correctly. I'll investigate further.
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.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Comment on attachment 9011844 [details] Bug 1493955 - Store floating-point preferences in a locale-independent way Nicholas Nethercote [:njn] has approved the revision.
Attachment #9011844 - Flags: review+
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.
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
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.
Pushed by gsvelto@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ba1fef7b14eb Store floating-point preferences in a locale-independent way r=njn
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
Flags: needinfo?(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.
Flags: needinfo?(gsvelto)
Pushed by gsvelto@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/326be6e41703 Store floating-point preferences in a locale-independent way r=njn
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: