[Webkit-unassigned] [Bug 6129] Incomplete implementation of CSS 2.1 system colors

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=6129


aroben at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #16501|review?                     |review-
               Flag|                            |




------- Comment #13 from aroben at apple.com  2007-10-27 23:10 PDT -------
(From update of attachment 16501)
+        else if (ident == CSS_VAL_GREY)
+            col = 0xFF808080;

Why not keep grey in the colorValues array?

+        color = [NSColor colorWithCalibratedRed:((float)pixel[0]) / 256.0
+                                          green:((float)pixel[1]) / 256.0
+                                           blue:((float)pixel[2]) / 256.0
+                                          alpha:((float)pixel[3]) / 256.0];
+        [offscreenRep release];
+    } 
+    return Color(static_cast<int>(256.0 * [color redComponent]),
static_cast<int>(256.0 * [color greenComponent]), static_cast<int>(256.0 *
[color blueComponent]));

I believe you should replace every occurrence of 256.0 with 255.0f.

+void RenderThemeMac::systemColor(int propId, Color& color) const
+{
+    if (m_systemColorCache.contains(propId))
+        color = m_systemColorCache.get(propId);

It would be good to return right here so the long switch statement doesn't have
to be in an else block.

r- so the above can be fixed. The design looks good overall. We're also going
to have to consider whether this will break existing web sites.


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