Bug 27133

Summary:

[Chromium] Upstream RGBColor

Product:

WebKit

Reporter:

Nate Chapin <japhet>

Component:

WebCore Misc.

Assignee:

Nate Chapin <japhet>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dglazkov, fishd, sam

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

27242

Attachments:

Description	Flags
patch	fishd: review-
patch2 - moved to WebCore/css/	sam: review+

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-	Details Formatted Diff Diff
patch2 - moved to WebCore/css/ (5.50 KB, patch) 2009-07-10 10:17 PDT, Nate Chapin	sam: review+	Details Formatted Diff Diff
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.