Last Comment Bug 816485 - layout.css.devPixelsPerPx parsed in locale-dependent manner
: layout.css.devPixelsPerPx parsed in locale-dependent manner
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla22
Assigned To: Jonathan Kew (:jfkthame)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-29 05:41 PST by Carl Nettelblad
Modified: 2013-02-26 08:03 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
interpreting the devPixelsPerPx value from about:config should not depend on locale (1.72 KB, patch)
2013-02-25 10:44 PST, Jonathan Kew (:jfkthame)
roc: review+
Details | Diff | Splinter Review

Description Carl Nettelblad 2012-11-29 05:41:05 PST
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20121113065533

Steps to reproduce:

In about:config, the property layout.css.devPixelsPerPx is of string type. The parsing of this (decimal) string is dependent on the current locale, i.e. 1.25 should be entered as 1,25 in a locale using comma as the decimal separator. This also means that a configuration transferred between locales might be parsed incorrectly.


Actual results:

1.25 is silently ignored as invalid, 1,25 works correctly - if current locale uses comma as the decimal separator.


Expected results:

Strings in configuration data should be locale agnostic.
Comment 1 Carl Nettelblad 2012-11-29 05:41:53 PST
This bug is also discussed in the comments in https://bugzilla.mozilla.org/show_bug.cgi?id=433664 , but only in passing.
Comment 2 Boris Zbarsky [:bz] 2012-11-29 08:43:53 PST
nsIWidget::GetDefaultScale does:

389     devPixelsPerCSSPixel = static_cast<float>(atof(prefString));
Comment 3 David Baron :dbaron: ⌚️UTC+1 (mostly busy through August 4; review requests must explain patch) 2012-12-22 11:55:31 PST
That's in:
http://hg.mozilla.org/mozilla-central/file/6e61c4a26591/widget/xpwidgets/nsBaseWidget.cpp#l380
Comment 4 Masatoshi Kimura [:emk] 2012-12-24 20:03:28 PST
Is PR_strtod the replacement?
Comment 5 Jonathan Kew (:jfkthame) 2013-02-25 10:44:26 PST
Created attachment 717961 [details] [diff] [review]
interpreting the devPixelsPerPx value from about:config should not depend on locale

Indeed, looks to me like PR_strtod would be better here. It only looks at the locale if nspr is built with USE_LOCALE defined, which we don't do AFAICT - but even in that case, inspecting the code shows that it would always understand '.' as decimal point, in addition to the locale's decimal_point if different. The downside of making this change, I suppose, would be that any user in a comma-decimal locale who has already modified the value (and figured out they needed to use comma) will then need to re-do their customization using the period. But I think we could live with that - after all, about:config comes with a health warning attached.
Comment 7 Ryan VanderMeulen [:RyanVM] 2013-02-26 08:03:01 PST
https://hg.mozilla.org/mozilla-central/rev/7ec9cda146fb

Note You need to log in before you can comment on or make changes to this bug.