[webkit-reviews] review denied: [Bug 26030] [Chromium] Chromium Linux ignores background color on <select>. : [Attachment 30881] patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Adam Langley <agl at chromium.org>'s
request for review:
Bug 26030: [Chromium] Chromium Linux ignores background color on <select>.
https://bugs.webkit.org/show_bug.cgi?id=26030

Attachment 30881: patch
https://bugs.webkit.org/attachment.cgi?id=30881&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Still has the { } style issues.

The .css file should be a real .css file on disk and go through the normal
processing scripts, no?


I think I would have written this:
+	 if (!o->style()->hasBackground()) {
+	     paint.setARGB(0xff, 0xe9, 0xe9, 0xe9);
+	 } else {
+	     paint.setColor(o->style()->backgroundColor().rgb());
+	 }

As something like:

static Color defaultButtonGrey(0xff, 0xe9, 0xe9, 0xe9);
Color buttonBackground = !o->style()->hasBackground() ?
o->style()->backgroundColor() : defaultButtonGrey;
paint.setColor(buttonBackground.rgb());

But this makes me wonder if CSS buttons (which do not go through the button
theme calls) will look correct w/o a background?