RESOLVED FIXED 27133
[Chromium] Upstream RGBColor 
https://bugs.webkit.org/show_bug.cgi?id=27133
Summary [Chromium] Upstream RGBColor
Nate Chapin
Reported 2009-07-09 15:58:19 PDT
See summary.
Attachments
patch (5.29 KB, patch)
2009-07-09 16:23 PDT, Nate Chapin 		fishd: review-
DetailsFormatted DiffDiff
patch2 - moved to WebCore/css/ (5.50 KB, patch)
2009-07-10 10:17 PDT, Nate Chapin 		sam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Nate Chapin
Comment 1 2009-07-09 16:23:39 PDT
Created attachment 32538 [details] patch
Darin Fisher (:fishd, Google)
Comment 2 2009-07-09 20:44:44 PDT
Comment on attachment 32538 [details] patch > Index: WebCore/bindings/v8/RGBColor.cpp ... > + unsigned int value = (m_rgbColor >> 16) & 0xFF; > + return CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER); nit: i think the prevailing style in webkit is to just use "unsigned" instead of "unsigned int" otherwise, R=me
Sam Weinig
Comment 3 2009-07-09 20:57:00 PDT
What is v8 specific about this new file? It seems like it should be in css/.
Darin Fisher (:fishd, Google)
Comment 4 2009-07-09 21:16:06 PDT
We discussed this on IRC. Moving this file to WebCore/css is a good idea since it may make sense in the future for the JSC bindings to use the same file.
Darin Fisher (:fishd, Google)
Comment 5 2009-07-09 21:16:38 PDT
Comment on attachment 32538 [details] patch
Nate Chapin
Comment 6 2009-07-10 10:17:23 PDT
Created attachment 32562 [details] patch2 - moved to WebCore/css/ Moved to css/ and renamed "unsigned int" to "unsigned". That was all that was required, right?
Eric Seidel (no email)
Comment 7 2009-07-10 10:35:05 PDT
Comment on attachment 32562 [details] patch2 - moved to WebCore/css/ This could use Color.h for data storage and then the code for accessing red/green/blue would be simpler (just m_color.red()), but maybe that's overkill? Otherwise this looks fine.
Sam Weinig
Comment 8 2009-07-10 14:50:28 PDT
Comment on attachment 32562 [details] patch2 - moved to WebCore/css/ > + > + class RGBColor : public RefCounted<RGBColor> { > + public: > + // TODO(ager): Make constructor private once codegenerator changes > + // have landed upstream. We prefer FIXME, instead of TODO(name).
Nate Chapin
Comment 9 2009-07-14 15:30:18 PDT
Landed as r45872.
Note You need to log in before you can comment on or make changes to this bug.