[Webkit-unassigned] [Bug 18994] LANG/LC_ALL influences the result of element.style.opacity

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=18994


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #41061|review?                     |review+
               Flag|                            |




--- Comment #78 from Darin Adler <darin at apple.com>  2009-10-12 15:23:08 PDT ---
(From update of attachment 41061)
> -FAIL: read 90010000px back as 9.001e+07px, read again as 0px
> +PASS: read 90010000px back as 90010000px, read again as 90010000px

This succeeds only because the large number isn't large enough, right? With a
still larger number we'd have some other problem. I'd like to have a test
showing that we still fail with even larger numbers.

>  This test verifies that reading a floating-point opacity from CSS attributes gets back a properly-formatted float. Improperly handling locales that cause decimals to be written as commas might break it.
>  
>  PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS
> +PASS

In the future, you should use the script-tests approach to build tests like
this. This style of tests that write out more about what they're testing and
why they pass that you get automatically from that are superior. A good example
of how to do this sort of test is
LayoutTests/fast/dom/HTMLFormElement/script-tests/elements-not-in-document.js
and you may be able to see from that how this test could have been done that
way.

> From the printf docs %.6g means you show up to 6 significant digits, but then
> somehow it doesn't happen when your number very is close to zero and they also
> arbitrarily chop off trailing digits if they are 0 even when they are
> significant.  This patch passes all the layout tests, but I had to change the
> precision on one, but I find it hard to care too much about it.

As I understand it, 0.00000123456 is an example with 6 significant digits.
Maybe you misunderstood what it meant. But exactly matching "%.6lg" isn't the
point. Matching what we used to do is a good idea, but matching other browsers
or matching specifications are even more important.

I think at this point the patch is OK. Here are areas for future improvement
and refinement:

    - check on how other browsers handle formatting fractional values in CSS;
how many digits of precision, etc.
    - figure out if we want to stick with the 6-digits-of-precision thing
forever, or if we should have a different formatting rule

    - eliminate the use of snprintf -- as I said, JavaScriptCore has a function
with the relevant code already, JSC::UString::from, so you could just port that
to live in String.cpp

    - change the test to use script-tests for better clarity
    - add quite a few more kinds of numbers to the test cases
    - test all the different types of units -- a test should fail if we made
any mistakes in the switch statements

r=me

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.