From 4d700e555a914c13c4cc14a3d843a354e386320c Mon Sep 17 00:00:00 2001 From: Gabriele Svelto Date: Thu, 27 Sep 2018 20:41:39 +0000 Subject: [PATCH] Bug 1493955 - Store floating-point preferences in a locale-independent way r=njn Differential Revision: https://phabricator.services.mozilla.com/D6796 --HG-- extra : moz-landing-system : lando --- modules/libpref/Preferences.cpp | 9 +++++--- modules/libpref/Preferences.h | 5 +++-- modules/libpref/test/gtest/Basics.cpp | 21 +++++++++++++++++++ .../antitracking/AntiTrackingCommon.cpp | 1 + .../extensions/ExtensionPolicyService.cpp | 1 + .../tests/gtest/TestCombinedStacks.cpp | 2 ++ toolkit/recordreplay/ipc/ChildIPC.cpp | 1 + xpcom/threads/Scheduler.cpp | 1 - xpcom/threads/Scheduler.h | 1 + 9 files changed, 36 insertions(+), 6 deletions(-) diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index c211fba48110..319580ca3ed6 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -2394,6 +2394,7 @@ nsPrefBranch::GetFloatPref(const char* aPrefName, float* aRetVal) nsAutoCString stringVal; nsresult rv = GetCharPref(aPrefName, stringVal); if (NS_SUCCEEDED(rv)) { + // ToFloat() does a locale-independent conversion. *aRetVal = stringVal.ToFloat(&rv); } @@ -5035,6 +5036,7 @@ Preferences::GetFloat(const char* aPrefName, nsAutoCString result; nsresult rv = Preferences::GetCString(aPrefName, result, aKind); if (NS_SUCCEEDED(rv)) { + // ToFloat() does a locale-independent conversion. *aResult = result.ToFloat(&rv); } return rv; @@ -5845,7 +5847,9 @@ static void SetPref_float(const char* aName, float aDefaultValue) { PrefValue value; - nsPrintfCString defaultValue("%f", aDefaultValue); + // Convert the value in a locale-independent way. + nsAutoCString defaultValue; + defaultValue.AppendFloat(aDefaultValue); value.mStringVal = defaultValue.get(); pref_SetPref(aName, PrefType::String, @@ -5973,8 +5977,7 @@ InitVarCachePref(const nsACString& aName, } } -// XXX: this will eventually become used -MOZ_MAYBE_UNUSED static void +static void InitVarCachePref(const nsACString& aName, float* aCache, float aDefaultValue, diff --git a/modules/libpref/Preferences.h b/modules/libpref/Preferences.h index 1e31b86c5363..a11e9ee46a74 100644 --- a/modules/libpref/Preferences.h +++ b/modules/libpref/Preferences.h @@ -19,7 +19,6 @@ #include "nsIObserver.h" #include "nsIPrefBranch.h" #include "nsIPrefService.h" -#include "nsPrintfCString.h" #include "nsString.h" #include "nsTArray.h" #include "nsWeakReference.h" @@ -309,7 +308,9 @@ public: float aValue, PrefValueKind aKind = PrefValueKind::User) { - return SetCString(aPrefName, nsPrintfCString("%f", aValue), aKind); + nsAutoCString value; + value.AppendFloat(aValue); + return SetCString(aPrefName, value, aKind); } static nsresult SetCString(const char* aPrefName, diff --git a/modules/libpref/test/gtest/Basics.cpp b/modules/libpref/test/gtest/Basics.cpp index 28291a619b66..bb14b0ff6843 100644 --- a/modules/libpref/test/gtest/Basics.cpp +++ b/modules/libpref/test/gtest/Basics.cpp @@ -4,6 +4,8 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include + #include "gtest/gtest.h" #include "mozilla/Preferences.h" @@ -34,3 +36,22 @@ TEST(PrefsBasics, Errors) ASSERT_FLOAT_EQ(Preferences::GetFloat("foo.float", 1.0f, PrefValueKind::User), 4.44f); } + +TEST(PrefsBasics, FloatConversions) +{ + // Set a global locale that uses the comma as the decimal separator. Since + // we can't tell which locales will be available on a machine the tests are + // executed only if the locale was set correctly. + const char* oldLocale = setlocale(LC_NUMERIC, "nl_NL"); + if (oldLocale != nullptr) { + Preferences::SetFloat("foo.float", 3.33f, PrefValueKind::Default); + Preferences::SetFloat("foo.float", 4.44f, PrefValueKind::User); + ASSERT_FLOAT_EQ( + Preferences::GetFloat("foo.float", 1.0f, PrefValueKind::Default), 3.33f); + ASSERT_FLOAT_EQ( + Preferences::GetFloat("foo.float", 1.0f, PrefValueKind::User), 4.44f); + + // Restore the original locale + setlocale(LC_NUMERIC, oldLocale); + } +} diff --git a/toolkit/components/antitracking/AntiTrackingCommon.cpp b/toolkit/components/antitracking/AntiTrackingCommon.cpp index 45d446f12101..0b07c3000258 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.cpp +++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp @@ -30,6 +30,7 @@ #include "nsIWebProgressListener.h" #include "nsNetUtil.h" #include "nsPIDOMWindow.h" +#include "nsPrintfCString.h" #include "nsScriptSecurityManager.h" #include "nsSandboxFlags.h" #include "prtime.h" diff --git a/toolkit/components/extensions/ExtensionPolicyService.cpp b/toolkit/components/extensions/ExtensionPolicyService.cpp index 93a406408394..288a3f666917 100644 --- a/toolkit/components/extensions/ExtensionPolicyService.cpp +++ b/toolkit/components/extensions/ExtensionPolicyService.cpp @@ -29,6 +29,7 @@ #include "nsILoadInfo.h" #include "nsIXULRuntime.h" #include "nsNetUtil.h" +#include "nsPrintfCString.h" #include "nsPIDOMWindow.h" #include "nsXULAppAPI.h" #include "nsQueryObject.h" diff --git a/toolkit/components/telemetry/tests/gtest/TestCombinedStacks.cpp b/toolkit/components/telemetry/tests/gtest/TestCombinedStacks.cpp index fa75ea5ace43..d736b9f5685c 100644 --- a/toolkit/components/telemetry/tests/gtest/TestCombinedStacks.cpp +++ b/toolkit/components/telemetry/tests/gtest/TestCombinedStacks.cpp @@ -4,6 +4,8 @@ #include "other/CombinedStacks.h" #include "other/ProcessedStack.h" +#include "nsPrintfCString.h" + using namespace mozilla::Telemetry; using namespace TelemetryTestHelpers; diff --git a/toolkit/recordreplay/ipc/ChildIPC.cpp b/toolkit/recordreplay/ipc/ChildIPC.cpp index ce2e6f58fc01..6dcb40258032 100644 --- a/toolkit/recordreplay/ipc/ChildIPC.cpp +++ b/toolkit/recordreplay/ipc/ChildIPC.cpp @@ -23,6 +23,7 @@ #include "InfallibleVector.h" #include "MemorySnapshot.h" +#include "nsPrintfCString.h" #include "ParentInternal.h" #include "ProcessRecordReplay.h" #include "ProcessRedirect.h" diff --git a/xpcom/threads/Scheduler.cpp b/xpcom/threads/Scheduler.cpp index d75d0dd0816d..c80b85516412 100644 --- a/xpcom/threads/Scheduler.cpp +++ b/xpcom/threads/Scheduler.cpp @@ -16,7 +16,6 @@ #include "mozilla/SchedulerGroup.h" #include "nsCycleCollector.h" #include "nsIThread.h" -#include "nsPrintfCString.h" #include "nsThread.h" #include "nsThreadManager.h" #include "PrioritizedEventQueue.h" diff --git a/xpcom/threads/Scheduler.h b/xpcom/threads/Scheduler.h index 241bd61d359e..2c3401a90850 100644 --- a/xpcom/threads/Scheduler.h +++ b/xpcom/threads/Scheduler.h @@ -14,6 +14,7 @@ #include "mozilla/UniquePtr.h" #include "nsTArray.h" #include "nsILabelableRunnable.h" +#include "nsPrintfCString.h" // Windows silliness. winbase.h defines an empty no-argument Yield macro. #undef Yield