2011-04-19 Naoki Takano <takano.naoki@gmail.com>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Apr 2011 05:16:35 +0000 (05:16 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Apr 2011 05:16:35 +0000 (05:16 +0000)
        Reviewed by Kent Tamura.

        [Chromium]UI polishes and tweaks to Autofill dropdown menu.
        https://bugs.webkit.org/show_bug.cgi?id=58505
        http://code.google.com/p/chromium/issues/detail?id=51077

        No new tests. Because this is autofill looking problem in Chromium.
        Add m_menuType as PopupMenuStyle to change popup style change.
        Put kLinePaddingHeight at the top and bottom of each line if m_menuType is AutofillPopup.
        Change separator color to #dcdcdc and remove sparatorPadding at the edge if m_menuType is AutofillPopup.
        Change the line height of separator as only the piece if m_menuType is AutofillPopup.
        Change the label font size 0.9 time smaller than regular font size if m_menuType is AutofillPopup.

        * platform/PopupMenuStyle.h: Add enum PopupMenuType;
        (WebCore::PopupMenuStyle::PopupMenuStyle): Add m_menuType.
        (WebCore::PopupMenuStyle::menuType): Add to change the style according to the return value.
        * platform/chromium/PopupMenuChromium.cpp:
        (WebCore::PopupListBox::paintRow):Change the separator color to #dcdcdc.
        Change the edge padding according to menuStyle().
        (WebCore::PopupListBox::getRowHeight): Add kLineHeightMargin*2 for each line height.
2011-04-19  Naoki Takano  <takano.naoki@gmail.com>

        Reviewed by Kent Tamura.

        [Chromium]UI polishes and tweaks to Autofill dropdown menu.
        https://bugs.webkit.org/show_bug.cgi?id=58505

        * src/AutoFillPopupMenuClient.cpp:
        (WebKit::AutoFillPopupMenuClient::initialize): Set AutofillPopup for menuStyle.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@84342 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/PopupMenuStyle.h
Source/WebCore/platform/chromium/PopupMenuChromium.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/AutoFillPopupMenuClient.cpp

index 64ce8ec..d041f17 100644 (file)
@@ -1,3 +1,26 @@
+2011-04-19  Naoki Takano  <takano.naoki@gmail.com>
+
+        Reviewed by Kent Tamura.
+
+        [Chromium]UI polishes and tweaks to Autofill dropdown menu.
+        https://bugs.webkit.org/show_bug.cgi?id=58505
+        http://code.google.com/p/chromium/issues/detail?id=51077
+
+        No new tests. Because this is autofill looking problem in Chromium.
+        Add m_menuType as PopupMenuStyle to change popup style change.
+        Put kLinePaddingHeight at the top and bottom of each line if m_menuType is AutofillPopup.
+        Change separator color to #dcdcdc and remove sparatorPadding at the edge if m_menuType is AutofillPopup.
+        Change the line height of separator as only the piece if m_menuType is AutofillPopup.
+        Change the label font size 0.9 time smaller than regular font size if m_menuType is AutofillPopup.
+
+        * platform/PopupMenuStyle.h: Add enum PopupMenuType;
+        (WebCore::PopupMenuStyle::PopupMenuStyle): Add m_menuType.
+        (WebCore::PopupMenuStyle::menuType): Add to change the style according to the return value.
+        * platform/chromium/PopupMenuChromium.cpp:
+        (WebCore::PopupListBox::paintRow):Change the separator color to #dcdcdc.
+        Change the edge padding according to menuStyle().
+        (WebCore::PopupListBox::getRowHeight): Add kLineHeightMargin*2 for each line height.
+
 2011-04-19  Simon Fraser  <simon.fraser@apple.com>
 
         Reviewed by Dan Bernstein.
index 3cb33cc..e979e25 100644 (file)
@@ -35,7 +35,8 @@ namespace WebCore {
 
 class PopupMenuStyle {
 public:
-    PopupMenuStyle(const Color& foreground, const Color& background, const Font& font, bool visible, bool isDisplayNone, Length textIndent, TextDirection textDirection, bool hasTextDirectionOverride)
+    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)
         : m_foregroundColor(foreground)
         , m_backgroundColor(background)
         , m_font(font)
@@ -44,9 +45,10 @@ public:
         , m_textIndent(textIndent)
         , m_textDirection(textDirection)
         , m_hasTextDirectionOverride(hasTextDirectionOverride)
+        , m_menuType(menuType)
     {
     }
-    
+
     const Color& foregroundColor() const { return m_foregroundColor; }
     const Color& backgroundColor() const { return m_backgroundColor; }
     const Font& font() const { return m_font; }
@@ -55,7 +57,7 @@ public:
     Length textIndent() const { return m_textIndent; }
     TextDirection textDirection() const { return m_textDirection; }
     bool hasTextDirectionOverride() const { return m_hasTextDirectionOverride; }
-
+    PopupMenuType menuType() const { return m_menuType; }
 private:
     Color m_foregroundColor;
     Color m_backgroundColor;
@@ -65,6 +67,7 @@ private:
     Length m_textIndent;
     TextDirection m_textDirection;
     bool m_hasTextDirectionOverride;
+    PopupMenuType m_menuType;
 };
 
 } // namespace WebCore
index af97ecf..80a66b2 100644 (file)
@@ -75,6 +75,7 @@ static const int kTextToLabelPadding = 10;
 static const int kLabelToIconPadding = 5;
 static const int kMinEndOfLinePadding = 2;
 static const TimeStamp kTypeAheadTimeoutMs = 1000;
+static const int kLinePaddingHeight = 3; // Padding height put at the top and bottom of each line.
 
 // The settings used for the drop down menu.
 // This is the delegate used if none is provided.
@@ -912,12 +913,15 @@ void PopupListBox::paintRow(GraphicsContext* gc, const IntRect& rect, int rowInd
 
     gc->fillRect(rowRect, backColor, ColorSpaceDeviceRGB);
 
+    // It doesn't look good but Autofill requires special style for separator.
+    // Autofill doesn't have padding and #dcdcdc color.
     if (m_popupClient->itemIsSeparator(rowIndex)) {
+        int padding = style.menuType() == PopupMenuStyle::AutofillPopup ? 0 : separatorPadding;
         IntRect separatorRect(
-            rowRect.x() + separatorPadding,
+            rowRect.x() + padding,
             rowRect.y() + (rowRect.height() - separatorHeight) / 2,
-            rowRect.width() - 2 * separatorPadding, separatorHeight);
-        gc->fillRect(separatorRect, textColor, ColorSpaceDeviceRGB);
+            rowRect.width() - 2 * padding, separatorHeight);
+        gc->fillRect(separatorRect, style.menuType() == PopupMenuStyle::AutofillPopup ? Color(0xdc, 0xdc, 0xdc) : textColor, ColorSpaceDeviceRGB);
         return;
     }
 
@@ -988,6 +992,16 @@ void PopupListBox::paintRow(GraphicsContext* gc, const IntRect& rect, int rowInd
     // Draw the the label if applicable.
     if (itemLabel.isEmpty())
         return;
+
+    // Autofill label is 0.9 smaller than regular font size.
+    if (style.menuType() == PopupMenuStyle::AutofillPopup) {
+        itemFont = m_popupClient->itemStyle(rowIndex).font();
+        FontDescription d = itemFont.fontDescription();
+        d.setComputedSize(d.computedSize() * 0.9);
+        itemFont = Font(d, itemFont.letterSpacing(), itemFont.wordSpacing());
+        itemFont.update(0);
+    }
+
     TextRun labelTextRun(itemLabel.characters(), itemLabel.length(), false, 0, 0, TextRun::AllowTrailingExpansion, rtl, style.hasTextDirectionOverride());
     if (rightAligned)
         textX = max(0, m_popupClient->clientPaddingLeft() - m_popupClient->clientInsetLeft());
@@ -1104,13 +1118,18 @@ int PopupListBox::getRowHeight(int index)
     if (m_popupClient->itemStyle(index).isDisplayNone())
         return 0;
 
+    // Separator row height is the same size as itself.
+    if (m_popupClient->itemIsSeparator(index))
+        return separatorHeight;
+
     String icon = m_popupClient->itemIcon(index);
     RefPtr<Image> image(Image::loadPlatformResource(icon.utf8().data()));
 
     int fontHeight = getRowFont(index).fontMetrics().height();
     int iconHeight = (image && !image->isNull()) ? image->rect().height() : 0;
 
-    return max(fontHeight, iconHeight);
+    int linePaddingHeight = m_popupClient->menuStyle().menuType() == PopupMenuStyle::AutofillPopup ? kLinePaddingHeight : 0;
+    return max(fontHeight, iconHeight) + linePaddingHeight * 2;
 }
 
 IntRect PopupListBox::getRowBounds(int index)
index 924799c..37d3421 100644 (file)
@@ -1,3 +1,13 @@
+2011-04-19  Naoki Takano  <takano.naoki@gmail.com>
+
+        Reviewed by Kent Tamura.
+
+        [Chromium]UI polishes and tweaks to Autofill dropdown menu.
+        https://bugs.webkit.org/show_bug.cgi?id=58505
+
+        * src/AutoFillPopupMenuClient.cpp:
+        (WebKit::AutoFillPopupMenuClient::initialize): Set AutofillPopup for menuStyle.
+
 2011-04-19  Dirk Pranke  <dpranke@chromium.org>
 
         Unreviewed, attempting build fix.
index d04c57c..c73e059 100644 (file)
@@ -291,9 +291,15 @@ void AutoFillPopupMenuClient::initialize(
     regularFont.update(textField->document()->styleSelector()->fontSelector());
     // The direction of text in popup menu is set the same as the direction of
     // the input element: textField.
-    m_regularStyle.set(new PopupMenuStyle(Color::black, Color::white, regularFont,
-                                          true, false, Length(WebCore::Fixed),
-                                          textField->renderer()->style()->direction(), textField->renderer()->style()->unicodeBidi() == Override));
+    m_regularStyle.set(new PopupMenuStyle(Color::black,
+                                          Color::white,
+                                          regularFont,
+                                          true,
+                                          false,
+                                          Length(WebCore::Fixed),
+                                          textField->renderer()->style()->direction(),
+                                          textField->renderer()->style()->unicodeBidi() == Override,
+                                          PopupMenuStyle::AutofillPopup));
 
     FontDescription warningFontDescription = regularFont.fontDescription();
     warningFontDescription.setItalic(true);
@@ -306,7 +312,8 @@ void AutoFillPopupMenuClient::initialize(
                                           m_regularStyle->isDisplayNone(),
                                           m_regularStyle->textIndent(),
                                           m_regularStyle->textDirection(),
-                                          m_regularStyle->hasTextDirectionOverride()));
+                                          m_regularStyle->hasTextDirectionOverride(),
+                                          PopupMenuStyle::AutofillPopup));
 }
 
 void AutoFillPopupMenuClient::setSuggestions(const WebVector<WebString>& names,