Last Comment Bug 669220 - cubic-bezier() computed style serialization outputs locale-dependent floats (test_value_storage.html fails in some locales)
: cubic-bezier() computed style serialization outputs locale-dependent floats (...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla8
Assigned To: arno renevier
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-04 14:30 PDT by arno renevier
Modified: 2011-07-07 04:49 PDT (History)
5 users (show)
Ms2ger: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 (3.37 KB, patch)
2011-07-05 06:12 PDT, arno renevier
no flags Details | Diff | Review
patch v2 (2.47 KB, patch)
2011-07-05 09:01 PDT, arno renevier
bzbarsky: review+
Details | Diff | Review
reviewed patch with a commit message (2.57 KB, patch)
2011-07-06 00:02 PDT, arno renevier
no flags Details | Diff | Review
reviewed patch with a commit message (2) (2.57 KB, patch)
2011-07-06 09:08 PDT, arno renevier
no flags Details | Diff | Review
reviewed patch with a commit message (2) (2.58 KB, patch)
2011-07-06 09:10 PDT, arno renevier
no flags Details | Diff | Review

Description arno renevier 2011-07-04 14:30:35 PDT
Hi,
I'm using french locale, and have 94 failing tests in test_value_storage.html, all related to -moz-animation-timing-function.
Example of a failing test:
failed | parse+compute+serialize(-moz-animation-timing-function) should be idempotent for '-moz-animation: bounce 1s linear 2s' - got "cubic-bezier(0,250000, 0,100000, 0,250000, 1,000000)", expected "cubic-bezier(0,000000, 0,000000, 1,000000, 1,000000)"

This happens because getComputedStyle uses printf with floats 
http://hg.mozilla.org/mozilla-central/file/320e5abc6f5a/layout/style/nsComputedDOMStyle.cpp#l3986
This gives me, with my locale, computed -moz-animation-timing-function which look like:

cubic-bezier(0,250000, 0,100000, 0,250000, 1,000000)

Which is wrong (notice the comma instead of dots for decimal separators).
Comment 1 Boris Zbarsky [:bz] 2011-07-04 19:50:29 PDT
Yeah, that's just wrong.  Arno, do you want to fix, or should I?
Comment 2 arno renevier 2011-07-05 06:12:09 PDT
Created attachment 543919 [details] [diff] [review]
patch v1

I'm unsure if this is the right way. Should I instead reset LC_NUMERIC locale before calling nsPrintfCString (and set it back after) ?
Comment 3 Robert Longson 2011-07-05 06:26:19 PDT
You could use nsAutoString and AppendPrintf e.g. AppendPrintf("%f,%f", x, y) to build up a string. Or alternatively nsTextFormatter::ssprintf e.g.

    nsTextFormatter::ssprintf(aValue,
                              NS_LITERAL_STRING("%g,%g").get(),
                              x, y);
Comment 4 Boris Zbarsky [:bz] 2011-07-05 07:37:14 PDT
Or just use AppendFloat().

You do NOT want to mess with LC_NUMERIC.
Comment 5 arno renevier 2011-07-05 09:01:44 PDT
Created attachment 543970 [details] [diff] [review]
patch v2
Comment 6 Boris Zbarsky [:bz] 2011-07-05 12:29:00 PDT
Comment on attachment 543970 [details] [diff] [review]
patch v2

r=me, but please add a useful commit message!
Comment 7 arno renevier 2011-07-06 00:02:58 PDT
Created attachment 544162 [details] [diff] [review]
reviewed patch with a commit message
Comment 8 Boris Zbarsky [:bz] 2011-07-06 08:52:33 PDT
That commit message needs to list the bug number (and reviewer, typically).

I recommend: "Bug 669220.  Use location-independent float-to-string conversions for timing function computed styles.  r=bzbarsky".
Comment 9 arno renevier 2011-07-06 09:08:42 PDT
Created attachment 544262 [details] [diff] [review]
reviewed patch with a commit message (2)
Comment 10 arno renevier 2011-07-06 09:10:10 PDT
Created attachment 544264 [details] [diff] [review]
reviewed patch with a commit message (2)
Comment 11 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-07-06 11:28:08 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9b83b5cfcab

(I changed "location-independent" in the commit message to "locale-independent".)
Comment 12 Marco Bonardo [::mak] 2011-07-07 03:22:12 PDT
http://hg.mozilla.org/mozilla-central/rev/a9b83b5cfcab

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