Closed Bug 1493955 Opened 3 years ago Closed 3 years ago

Floating-point conversions in preferences are not locale-independent


(Core :: Preferences: Backend, defect)

Not set



Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- unaffected
firefox64 --- fixed


(Reporter: gsvelto, Assigned: gsvelto)



(Keywords: regression)


(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
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+
Duplicate of this bug: 1494002
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:
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
Store floating-point preferences in a locale-independent way r=njn
Backed out changeset ba1fef7b14eb (Bug 1493955) for GTest failures.

Push with failures:

Backout link:

Failure log:

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
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
15:38:32    ERROR - shutil error: Destination path '/Users/cltbld/tasks/task_1538087754/build/application/Firefox' 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
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
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
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
15:38:32     INFO - mkdir: /Users/cltbld/tasks/task_1538087754/build/application/Firefox
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
15:38:32     INFO - rmtree: /Users/cltbld/tasks/task_1538087754/build/application/Firefox
15:38:32     INFO - retry: Calling rmtree with args: ('/Users/cltbld/tasks/task_1538087754/build/application/Firefox',), 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
Store floating-point preferences in a locale-independent way r=njn
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Duplicate of this bug: 1494002
You need to log in before you can comment on or make changes to this bug.