Changeset 130513 in webkit

Timestamp:

Oct 5, 2012, 9:15:35 AM (13 years ago)

Author:

commit-queue@webkit.org

Message:

[chromium] Only inflate the height of rows in a popup menu when a touch device is detected.
​https://bugs.webkit.org/show_bug.cgi?id=98515

Patch by Kevin Ellis <​kevers@chromium.org> on 2012-10-05
Reviewed by Adam Barth.

Enforces a minimum row height for popup menus when a touch device is
detected. In a previous patch (r127597), the sizing of popup was
consolidated for touch and non-touch. Based on user feedback, reverting
to the old behavior for non-touch and only adding padding for touch
devices seems like a much safer strategy. This patch is not a direct
revert of r127567 since the padding previously used for touch is a bit
excessive.

Covered by existing tests.

• platform/chromium/PopupListBox.cpp:

(WebCore::PopupListBox::getRowHeight):

• platform/chromium/PopupMenuChromium.cpp:

(WebCore):

• platform/chromium/PopupMenuChromium.h:

(WebCore::PopupMenuChromium::optionRowHeightForTouch):
(WebCore::PopupMenuChromium::setOptionRowHeightForTouch):
(PopupMenuChromium):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/chromium/PopupListBox.cpp (modified) (2 diffs)
• platform/chromium/PopupMenuChromium.cpp (modified) (1 diff)
• platform/chromium/PopupMenuChromium.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-05  Kevin Ellis  <kevers@chromium.org>
+
+        [chromium] Only inflate the height of rows in a popup menu when a touch device is detected.
+        https://bugs.webkit.org/show_bug.cgi?id=98515
+
+        Reviewed by Adam Barth.
+
+        Enforces a minimum row height for popup menus when a touch device is
+        detected.  In a previous patch (r127597), the sizing of popup was 
+        consolidated for touch and non-touch.  Based on user feedback, reverting
+        to the old behavior for non-touch and only adding padding for touch
+        devices seems like a much safer strategy.  This patch is not a direct
+        revert of r127567 since the padding previously used for touch is a bit
+        excessive.
+
+        Covered by existing tests.
+
+        * platform/chromium/PopupListBox.cpp:
+        (WebCore::PopupListBox::getRowHeight):
+        * platform/chromium/PopupMenuChromium.cpp:
+        (WebCore):
+        * platform/chromium/PopupMenuChromium.h:
+        (WebCore::PopupMenuChromium::optionRowHeightForTouch):
+        (WebCore::PopupMenuChromium::setOptionRowHeightForTouch):
+        (PopupMenuChromium):
+
 2012-10-05  Alexander Pavlov  <apavlov@chromium.org>
 

trunk/Source/WebCore/platform/chromium/PopupListBox.cpp

@@ -615 +615 @@
 int PopupListBox::getRowHeight(int index)
 {
+    int minimumHeight = PopupMenuChromium::minimumRowHeight();
+    if (m_settings.deviceSupportsTouch)
+        minimumHeight = max(minimumHeight, PopupMenuChromium::optionRowHeightForTouch());
+
     if (index < 0 || m_popupClient->itemStyle(index).isDisplayNone())
-        return PopupMenuChromium::minimumRowHeight();
+        return minimumHeight;
 
     // Separator row height is the same size as itself.
     if (m_popupClient->itemIsSeparator(index))
-        return max(separatorHeight, (PopupMenuChromium::minimumRowHeight()));
+        return max(separatorHeight, minimumHeight);
 
     String icon = m_popupClient->itemIcon(index);
@@ -630 +634 @@
     int linePaddingHeight = m_popupClient->menuStyle().menuType() == PopupMenuStyle::AutofillPopup ? kLinePaddingHeight : 0;
     int calculatedRowHeight = max(fontHeight, iconHeight) + linePaddingHeight * 2;
-    return max(calculatedRowHeight, PopupMenuChromium::minimumRowHeight());
+    return max(calculatedRowHeight, minimumHeight);
 }
 

trunk/Source/WebCore/platform/chromium/PopupMenuChromium.cpp

@@ -41 +41 @@
 namespace WebCore {
 
-int PopupMenuChromium::s_minimumRowHeight = 28;
+int PopupMenuChromium::s_minimumRowHeight = 0;
+int PopupMenuChromium::s_optionRowHeightForTouch = 28;
 
 // The settings used for the drop down menu.

trunk/Source/WebCore/platform/chromium/PopupMenuChromium.h

@@ -57 +57 @@
     static int minimumRowHeight() { return s_minimumRowHeight; }
     static void setMinimumRowHeight(int minimumRowHeight) { s_minimumRowHeight = minimumRowHeight; }
+    static int optionRowHeightForTouch() { return s_optionRowHeightForTouch; }
+    static void setOptionRowHeightForTouch(int optionRowHeightForTouch) { s_optionRowHeightForTouch = optionRowHeightForTouch; }
 
 private:
@@ -65 +67 @@
 
     static int s_minimumRowHeight;
+    static int s_optionRowHeightForTouch;
 };