[webkit-reviews] review denied: [Bug 32048] [chromium] theme scrollbars to match gtk theme : [Attachment 44125] patch with fixed tabs and caps

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Evan Martin <evan at chromium.org>'s
request for review:
Bug 32048: [chromium] theme scrollbars to match gtk theme
https://bugs.webkit.org/show_bug.cgi?id=32048

Attachment 44125: patch with fixed tabs and caps
https://bugs.webkit.org/attachment.cgi?id=44125&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Would be nice to document the pixel format.  "unsigned" doesn't tell the reader
much.  I'm kinda surprised we don't have a WebColor with all the other Web*
we've got. :)

I'm surprised we don't already ahve something like saturateAndBrighten in
Color.h

I would have written:
 86	if (color[1] > 1.0)
 87	    color[1] = 1.0;
 88	else if (color[1] < 0.0)
 89	    color[1] = 0.0;

as:
color[1] = std::min(std::max(hsv[1] + saturateAmount, 0.0), 1.0)

Ha.  If you don't like my min/max above, at least write a function do do it,
cause you repeat the same 4 lines everywehre:
if (min_diff < 0.2)
 131	     min_diff = 0.2;
 132	 else if (min_diff > 0.5)
 133	     min_diff = 0.5;

Style viloations:
inactive_color

I'm surprised check-webkit-style doesn't get those.  I guess I'll file a bug.

Otherwise looks good!