Bug 94625 - Create CSS color output string on 8 bits
Summary: Create CSS color output string on 8 bits
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-21 12:15 PDT by Benjamin Poulain
Modified: 2012-08-21 17:19 PDT (History)
5 users (show)

See Also:

Attachments
Patch (3.52 KB, patch)
2012-08-21 12:23 PDT, Benjamin Poulain 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2012-08-21 12:15:35 PDT 
Forcing 16 bits on everyone is bad.
Comment 1 Benjamin Poulain 2012-08-21 12:23:04 PDT 
Created attachment 159735 [details]
Patch
Comment 2 Andreas Kling 2012-08-21 13:26:13 PDT 
Comment on attachment 159735 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=159735&action=review

r=me

> Source/WebCore/css/CSSPrimitiveValue.cpp:1011
> +            bool colorHasAlpha = color.hasAlpha();

Does this really need a local variable?

> Source/WebCore/css/CSSPrimitiveValue.cpp:1029
> +                NumberToStringBuffer buffer;
> +                const char* alphaString = numberToFixedPrecisionString(color.alpha() / 255.0f, 6, buffer, true);
> +                result.append(alphaString, strlen(alphaString));

Too bad we need the strlen() here.
Comment 3 Benjamin Poulain 2012-08-21 14:22:38 PDT 
Comment on attachment 159735 [details]
Patch

Clearing flags on attachment: 159735

Committed r126186: <http://trac.webkit.org/changeset/126186>
Comment 4 Benjamin Poulain 2012-08-21 14:22:40 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 2012-08-21 14:35:56 PDT 
Shouldn’t we be doing this with StringBuilder rather than Vector<LChar>?
Comment 6 Benjamin Poulain 2012-08-21 17:19:02 PDT 
(In reply to comment #5)
> Shouldn’t we be doing this with StringBuilder rather than Vector<LChar>?

StringBuilder could do the job but it does not anything similar to WTF::appendNumber(). I do not mind updating StringBuilder if you think it is important.