Diff
Modified: trunk/Source/WebCore/ChangeLog (145405 => 145406)
--- trunk/Source/WebCore/ChangeLog 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebCore/ChangeLog 2013-03-11 21:35:18 UTC (rev 145406)
@@ -1,3 +1,41 @@
+2013-03-11 Xiyuan Xia <[email protected]>
+
+ [Chromium] chromium/linux breaks expectation of select popup background due to bad UA css rules
+ https://bugs.webkit.org/show_bug.cgi?id=111873
+
+ Reviewed by Tony Chang.
+
+ On linux the default <select> background color is too dark to use as the
+ popup background color. Last fixes:
+ https://bugs.webkit.org/show_bug.cgi?id=54115 and
+ https://bugs.webkit.org/show_bug.cgi?id=56023
+ attempt to fix the problem by applying a lighter background using
+ special <option> selector. This breaks expectations of some websites.
+
+ This CL reverts the bad UA css rules above and provides the lighter
+ background color if <select> and <option> elements are using the default
+ background.
+
+ No new tests, this tests <select> popups and can be verified by ManualTests/select-scroll.html.
+
+ * css/themeChromiumLinux.css:
+ (select):
+ * platform/PopupMenuStyle.h:
+ (WebCore::PopupMenuStyle::PopupMenuStyle):
+ (WebCore::PopupMenuStyle::backgroundColorType):
+ (PopupMenuStyle):
+ * platform/chromium/PopupListBox.cpp:
+ (WebCore::PopupListBox::paintRow):
+ * rendering/RenderMenuList.cpp:
+ (WebCore::RenderMenuList::itemStyle):
+ (WebCore::RenderMenuList::getItemBackgroundColor):
+ * rendering/RenderMenuList.h:
+ (RenderMenuList):
+ * rendering/RenderSearchField.cpp:
+ (WebCore::RenderSearchField::menuStyle):
+ * rendering/RenderThemeChromiumDefault.cpp:
+ (WebCore::RenderThemeChromiumDefault::systemColor):
+
2013-03-11 James Robinson <[email protected]>
Compile fix. Rubber-stamp by Eric Seidel.
Modified: trunk/Source/WebCore/css/themeChromiumLinux.css (145405 => 145406)
--- trunk/Source/WebCore/css/themeChromiumLinux.css 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebCore/css/themeChromiumLinux.css 2013-03-11 21:35:18 UTC (rev 145406)
@@ -31,15 +31,10 @@
/* These styles override other user-agent styles for Chromium on Linux. */
select {
- background-color: #dddddd;
+ /* Selects look like buttons with a drop down triangle on Linux. */
+ background-color: ButtonFace;
}
-select:not([size]):not([multiple]) option,
-select[size="0"] option,
-select[size="1"] option {
- background-color: #f7f7f7;
-}
-
input[type=range] {
color: #9d968E;
}
Modified: trunk/Source/WebCore/platform/PopupMenuStyle.h (145405 => 145406)
--- trunk/Source/WebCore/platform/PopupMenuStyle.h 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebCore/platform/PopupMenuStyle.h 2013-03-11 21:35:18 UTC (rev 145406)
@@ -36,7 +36,8 @@
class PopupMenuStyle {
public:
enum PopupMenuType { SelectPopup, AutofillPopup };
- PopupMenuStyle(const Color& foreground, const Color& background, const Font& font, bool visible, bool isDisplayNone, Length textIndent, TextDirection textDirection, bool hasTextDirectionOverride, PopupMenuType menuType = SelectPopup)
+ enum BackgroundColorType { DefaultBackgroundColor, CustomBackgroundColor };
+ PopupMenuStyle(const Color& foreground, const Color& background, const Font& font, bool visible, bool isDisplayNone, Length textIndent, TextDirection textDirection, bool hasTextDirectionOverride, BackgroundColorType backgroundColorType = DefaultBackgroundColor, PopupMenuType menuType = SelectPopup)
: m_foregroundColor(foreground)
, m_backgroundColor(background)
, m_font(font)
@@ -45,6 +46,7 @@
, m_textIndent(textIndent)
, m_textDirection(textDirection)
, m_hasTextDirectionOverride(hasTextDirectionOverride)
+ , m_backgroundColorType(backgroundColorType)
, m_menuType(menuType)
{
}
@@ -57,6 +59,7 @@
Length textIndent() const { return m_textIndent; }
TextDirection textDirection() const { return m_textDirection; }
bool hasTextDirectionOverride() const { return m_hasTextDirectionOverride; }
+ BackgroundColorType backgroundColorType() const { return m_backgroundColorType; }
PopupMenuType menuType() const { return m_menuType; }
private:
Color m_foregroundColor;
@@ -67,6 +70,7 @@
Length m_textIndent;
TextDirection m_textDirection;
bool m_hasTextDirectionOverride;
+ BackgroundColorType m_backgroundColorType;
PopupMenuType m_menuType;
};
Modified: trunk/Source/WebCore/platform/chromium/PopupListBox.cpp (145405 => 145406)
--- trunk/Source/WebCore/platform/chromium/PopupListBox.cpp 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebCore/platform/chromium/PopupListBox.cpp 2013-03-11 21:35:18 UTC (rev 145406)
@@ -30,6 +30,7 @@
#include "config.h"
#include "PopupListBox.h"
+#include "CSSValueKeywords.h"
#include "Font.h"
#include "FontSelector.h"
#include "FramelessScrollViewClient.h"
@@ -401,6 +402,16 @@
} else {
backColor = style.backgroundColor();
textColor = style.foregroundColor();
+
+#if OS(LINUX)
+ // On other platforms, the <option> background color is the same as the
+ // <select> background color. On Linux, that makes the <option>
+ // background color very dark, so by default, try to use a lighter
+ // background color for <option>s.
+ if (style.backgroundColorType() == PopupMenuStyle::DefaultBackgroundColor && RenderTheme::defaultTheme()->systemColor(CSSValueButtonface) == backColor)
+ backColor = RenderTheme::defaultTheme()->systemColor(CSSValueMenu);
+#endif
+
// FIXME: for now the label color is hard-coded. It should be added to
// the PopupMenuStyle.
labelColor = Color(115, 115, 115);
Modified: trunk/Source/WebCore/rendering/RenderMenuList.cpp (145405 => 145406)
--- trunk/Source/WebCore/rendering/RenderMenuList.cpp 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebCore/rendering/RenderMenuList.cpp 2013-03-11 21:35:18 UTC (rev 145406)
@@ -463,33 +463,46 @@
listIndex = 0;
}
HTMLElement* element = listItems[listIndex];
-
+
+ Color itemBackgroundColor;
+ bool itemHasCustomBackgroundColor;
+ getItemBackgroundColor(listIndex, itemBackgroundColor, itemHasCustomBackgroundColor);
+
RenderStyle* style = element->renderStyle() ? element->renderStyle() : element->computedStyle();
- return style ? PopupMenuStyle(style->visitedDependentColor(CSSPropertyColor), itemBackgroundColor(listIndex), style->font(), style->visibility() == VISIBLE,
- style->display() == NONE, style->textIndent(), style->direction(), isOverride(style->unicodeBidi())) : menuStyle();
+ return style ? PopupMenuStyle(style->visitedDependentColor(CSSPropertyColor), itemBackgroundColor, style->font(), style->visibility() == VISIBLE,
+ style->display() == NONE, style->textIndent(), style->direction(), isOverride(style->unicodeBidi()),
+ itemHasCustomBackgroundColor ? PopupMenuStyle::CustomBackgroundColor : PopupMenuStyle::DefaultBackgroundColor) : menuStyle();
}
-Color RenderMenuList::itemBackgroundColor(unsigned listIndex) const
+void RenderMenuList::getItemBackgroundColor(unsigned listIndex, Color& itemBackgroundColor, bool& itemHasCustomBackgroundColor) const
{
const Vector<HTMLElement*>& listItems = selectElement()->listItems();
- if (listIndex >= listItems.size())
- return style()->visitedDependentColor(CSSPropertyBackgroundColor);
+ if (listIndex >= listItems.size()) {
+ itemBackgroundColor = style()->visitedDependentColor(CSSPropertyBackgroundColor);
+ itemHasCustomBackgroundColor = false;
+ return;
+ }
HTMLElement* element = listItems[listIndex];
Color backgroundColor;
if (element->renderStyle())
backgroundColor = element->renderStyle()->visitedDependentColor(CSSPropertyBackgroundColor);
+ itemHasCustomBackgroundColor = backgroundColor.isValid() && backgroundColor.alpha();
// If the item has an opaque background color, return that.
- if (!backgroundColor.hasAlpha())
- return backgroundColor;
+ if (!backgroundColor.hasAlpha()) {
+ itemBackgroundColor = backgroundColor;
+ return;
+ }
// Otherwise, the item's background is overlayed on top of the menu background.
backgroundColor = style()->visitedDependentColor(CSSPropertyBackgroundColor).blend(backgroundColor);
- if (!backgroundColor.hasAlpha())
- return backgroundColor;
+ if (!backgroundColor.hasAlpha()) {
+ itemBackgroundColor = backgroundColor;
+ return;
+ }
// If the menu background is not opaque, then add an opaque white background behind.
- return Color(Color::white).blend(backgroundColor);
+ itemBackgroundColor = Color(Color::white).blend(backgroundColor);
}
PopupMenuStyle RenderMenuList::menuStyle() const
Modified: trunk/Source/WebCore/rendering/RenderMenuList.h (145405 => 145406)
--- trunk/Source/WebCore/rendering/RenderMenuList.h 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebCore/rendering/RenderMenuList.h 2013-03-11 21:35:18 UTC (rev 145406)
@@ -115,7 +115,7 @@
virtual bool hasLineIfEmpty() const { return true; }
- Color itemBackgroundColor(unsigned listIndex) const;
+ void getItemBackgroundColor(unsigned listIndex, Color&, bool& itemHasCustomBackgroundColor) const;
void createInnerBlock();
void adjustInnerStyle();
Modified: trunk/Source/WebCore/rendering/RenderSearchField.cpp (145405 => 145406)
--- trunk/Source/WebCore/rendering/RenderSearchField.cpp 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebCore/rendering/RenderSearchField.cpp 2013-03-11 21:35:18 UTC (rev 145406)
@@ -266,7 +266,7 @@
PopupMenuStyle RenderSearchField::menuStyle() const
{
return PopupMenuStyle(style()->visitedDependentColor(CSSPropertyColor), style()->visitedDependentColor(CSSPropertyBackgroundColor), style()->font(), style()->visibility() == VISIBLE,
- style()->display() == NONE, style()->textIndent(), style()->direction(), isOverride(style()->unicodeBidi()));
+ style()->display() == NONE, style()->textIndent(), style()->direction(), isOverride(style()->unicodeBidi()), PopupMenuStyle::CustomBackgroundColor);
}
int RenderSearchField::clientInsetLeft() const
Modified: trunk/Source/WebCore/rendering/RenderThemeChromiumDefault.cpp (145405 => 145406)
--- trunk/Source/WebCore/rendering/RenderThemeChromiumDefault.cpp 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebCore/rendering/RenderThemeChromiumDefault.cpp 2013-03-11 21:35:18 UTC (rev 145406)
@@ -91,9 +91,12 @@
Color RenderThemeChromiumDefault::systemColor(int cssValueId) const
{
static const Color defaultButtonGrayColor(0xffdddddd);
+ static const Color defaultMenuColor(0xfff7f7f7);
if (cssValueId == CSSValueButtonface)
return defaultButtonGrayColor;
+ if (cssValueId == CSSValueMenu)
+ return defaultMenuColor;
return RenderTheme::systemColor(cssValueId);
}
Modified: trunk/Source/WebKit/chromium/ChangeLog (145405 => 145406)
--- trunk/Source/WebKit/chromium/ChangeLog 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebKit/chromium/ChangeLog 2013-03-11 21:35:18 UTC (rev 145406)
@@ -1,3 +1,15 @@
+2013-03-11 Xiyuan Xia <[email protected]>
+
+ [Chromium] chromium/linux breaks expectation of select popup background due to bad UA css rules
+ https://bugs.webkit.org/show_bug.cgi?id=111873
+
+ Reviewed by Tony Chang.
+
+ Update PopupMenuStyle constructor call sites.
+
+ * src/AutofillPopupMenuClient.cpp:
+ (WebKit::AutofillPopupMenuClient::initialize):
+
2013-03-11 Abhishek Arya <[email protected]>
Add ASSERT_WITH_SECURITY_IMPLICATION to catch bad casts.
Modified: trunk/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp (145405 => 145406)
--- trunk/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp 2013-03-11 21:26:30 UTC (rev 145405)
+++ trunk/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp 2013-03-11 21:35:18 UTC (rev 145406)
@@ -289,19 +289,19 @@
// The direction of text in popup menu is set the same as the direction of
// the input element: textField.
m_regularStyle = adoptPtr(new PopupMenuStyle(Color::black, Color::white, regularFont, true, false,
- Length(WebCore::Fixed), textField->renderer()->style()->direction(),
- textField->renderer()->style()->unicodeBidi() == Override,
- PopupMenuStyle::AutofillPopup));
+ Length(WebCore::Fixed), textField->renderer()->style()->direction(),
+ textField->renderer()->style()->unicodeBidi() == Override,
+ PopupMenuStyle::CustomBackgroundColor, PopupMenuStyle::AutofillPopup));
FontDescription warningFontDescription = regularFont.fontDescription();
warningFontDescription.setItalic(true);
Font warningFont(warningFontDescription, regularFont.letterSpacing(), regularFont.wordSpacing());
warningFont.update(regularFont.fontSelector());
m_warningStyle = adoptPtr(new PopupMenuStyle(Color::darkGray, m_regularStyle->backgroundColor(), warningFont,
- m_regularStyle->isVisible(), m_regularStyle->isDisplayNone(),
- m_regularStyle->textIndent(), m_regularStyle->textDirection(),
- m_regularStyle->hasTextDirectionOverride(),
- PopupMenuStyle::AutofillPopup));
+ m_regularStyle->isVisible(), m_regularStyle->isDisplayNone(),
+ m_regularStyle->textIndent(), m_regularStyle->textDirection(),
+ m_regularStyle->hasTextDirectionOverride(),
+ PopupMenuStyle::CustomBackgroundColor, PopupMenuStyle::AutofillPopup));
}
void AutofillPopupMenuClient::setSuggestions(const WebVector<WebString>& names,
