[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





--- Comment #120 from Nikolas Zimmermann <zimmermann at kde.org>  2010-10-14 05:15:11 PST ---
(In reply to comment #118)

Here are some news, for those that don't follow all webkit commits:

> > Excellent, that would be really handy. Do you have already started coding this?
> 
> I haven't, but this should be pretty trivial:
> 
> String String::cat(const String& s1, const String& s2)
..

This is obsolete now: we have wtf/text/StringConcatenate, solving this problem in an elegant and performant way.

> And so on for more more arguments.  One problem, I think StringBuilder is currently in WebCore, not WTF, if should move to wtf/text with the rest of the string code.  (I have a patch I'm working on to rewrite StringBuilder, so we may conflict a little there, but if you're just moving the code should be easy to resolve for whoever lands second!)
StringBuilder lives in wtf/text now, but using the more performant implementation based on runtime/StringBuilder.

> 
> > SVGs requirement is uncommon right, but we don't want this for the render path dumps, we definately want exponential numbers in the SVG DRT dumps.
> 
> Ah I see, yes okay.  so for these cases the HTML5/ECMAScript behaviour should probably be okay?
Exactly, the only drawback is that each expected.txt file is affected, but I think we can live with that, instead of inventing extra hacks (remove trailing zeros etc..) just for the sake of maintaining compatibility between the new & current DRT dump style.

> > Excellent! I'm happy to work on this, I hope you already wrote the String::cat stuff? If yes, I'd work on a String::number() implementation (like the one in my attached patch) which gives us more flexibility (add an FloatingPointConversionMode enum, with AvoidExponentials, RoundSiginifcantFigures, RoundDecimalPlaces modes, that can be OR'ed together, etc..)
> 
> My main concern here would be that UString::number can be a hot function, so it would probably be undesirable to add unnecessary overhead to switch on a parameter to check the conversion type.  Also, there may be some subtleties to the requirements of any other conversion functions, so we may be better adding them as we need them.  For example, the decimal number conversion uses a fixed size buffer on the stack, which would not be safe to convert large double values to decimal format.  To safely use this conversion the simple solution would probably be to truncate values into a range (which I believe is explicitly permitted in the SVG spec, Simon?) – however String::number having this truncation built in would be a little weird and arbirtary.  Also, to-fixed methods may require a precision argument.

I made up my mind, and think we should have a simple String::number(double) method which serializes the double using the numberToString() method (which uses DecimalNumber) to match the HTML5/ECMAScript default way of doing this. I'm concerned as well to add any extra logic, given the fact we ideally want to switch from UString to WTFString soon.

We can add another method, which takes a precision argument, with explicit naming, whenever we need it (String::numberRoundingSignificantFigures(number, 6)) etc..

> 
> So perhaps it would just be best to switch String::number to have the HTML5/ECMAScript behaviour (matching UString::number) for all uses, for now?
Definately.

I'm still preparing patches, before I can actually work on fixing String::number.
My current plan is:
1) Remove all "simple" String::format usage (no floating-point formatting involved, no pointer dumps) from all of WebCore/WebKit - replace by StringConcatenate (see bug 47664)

2) Identify a common set of missing methods that would be needed to replace the rest of the String::format usages (String::numberAsHex, etc.)

3) Implement them, kill String::format completly.

4) Be happy! :-)

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