Floating-point conversions in preferences are not locale-independent

RESOLVED FIXED in Firefox 64

Status

()

defect
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: gsvelto, Assigned: gsvelto)

Tracking

({regression})

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox62 unaffected, firefox63 unaffected, firefox64 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

8 months ago
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 '.'.
Assignee

Comment 1

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

Comment 2

8 months ago
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

Updated

8 months ago
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+
Duplicate of this bug: 1494002
Assignee

Comment 6

8 months ago
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.
Assignee

Comment 8

8 months ago
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
Assignee

Comment 9

8 months ago
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

8 months ago
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)
Assignee

Comment 12

8 months ago
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)

Comment 13

8 months ago
Pushed by gsvelto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/326be6e41703
Store floating-point preferences in a locale-independent way r=njn

Comment 14

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/326be6e41703
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months 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.