Changeset 110359 in webkit

Timestamp:

Mar 9, 2012 6:40:26 PM (12 years ago)

Author:

commit-queue@webkit.org

Message:

[chromium] Increase size of Combo Box Options for touch and high DPI devices
​https://bugs.webkit.org/show_bug.cgi?id=80027

Patch by Tim Dresser <​tdresser@chromium.org> on 2012-03-09
Reviewed by Darin Fisher.

Source/WebCore:

Scale Combo box popups by defaultDeviceScaleFactor, and add padding to
<option> elements when touch is enabled.

Manually tested with --default-device-scale-factor=1,2 and unset.
Each of these were tested with RuntimeEnabledFeatures::touchEnabled
set to true and false.

• platform/chromium/PopupListBox.cpp:

(WebCore::PopupListBox::paint):
(WebCore::PopupListBox::paintRow):
(WebCore::PopupListBox::getRowHeight):

• platform/chromium/PopupListBox.h:

(PopupContainerSettings):

• platform/chromium/PopupMenuChromium.cpp:

(WebCore):
(WebCore::PopupMenuChromium::show):

• platform/chromium/PopupMenuChromium.h:

(WebCore::PopupMenuChromium::optionPaddingForTouch):
(WebCore::PopupMenuChromium::setOptionPaddingForTouch):
(PopupMenuChromium):

• rendering/RenderMenuList.cpp:

(WebCore::RenderMenuList::showPopup):

Source/WebKit/chromium:

• src/WebViewImpl.cpp:

(WebKit::WebViewImpl::gestureEvent):
(WebKit::WebViewImpl::applyAutofillSuggestions):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/platform/chromium/PopupListBox.cpp (modified) (6 diffs)
• WebCore/platform/chromium/PopupListBox.h (modified) (1 diff)
• WebCore/platform/chromium/PopupMenuChromium.cpp (modified) (2 diffs)
• WebCore/platform/chromium/PopupMenuChromium.h (modified) (2 diffs)
• WebCore/rendering/RenderMenuList.cpp (modified) (2 diffs)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/WebViewImpl.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-03-09  Tim Dresser  <tdresser@chromium.org>
+
+        [chromium] Increase size of Combo Box Options for touch and high DPI devices
+        https://bugs.webkit.org/show_bug.cgi?id=80027
+
+        Reviewed by Darin Fisher.
+
+        Scale Combo box popups by defaultDeviceScaleFactor, and add padding to
+        <option> elements when touch is enabled.
+
+        Manually tested with --default-device-scale-factor=1,2 and unset.
+        Each of these were tested with RuntimeEnabledFeatures::touchEnabled
+        set to true and false.
+
+        * platform/chromium/PopupListBox.cpp:
+        (WebCore::PopupListBox::paint):
+        (WebCore::PopupListBox::paintRow):
+        (WebCore::PopupListBox::getRowHeight):
+        * platform/chromium/PopupListBox.h:
+        (PopupContainerSettings):
+        * platform/chromium/PopupMenuChromium.cpp:
+        (WebCore):
+        (WebCore::PopupMenuChromium::show):
+        * platform/chromium/PopupMenuChromium.h:
+        (WebCore::PopupMenuChromium::optionPaddingForTouch):
+        (WebCore::PopupMenuChromium::setOptionPaddingForTouch):
+        (PopupMenuChromium):
+        * rendering/RenderMenuList.cpp:
+        (WebCore::RenderMenuList::showPopup):
+
 2012-03-09  Stephen White  <senorblanco@chromium.org>
 

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

@@ -45 +45 @@
 #include "PopupMenuClient.h"
 #include "RenderTheme.h"
+#include "RuntimeEnabledFeatures.h"
 #include "ScrollbarTheme.h"
 #include "StringTruncator.h"
@@ -353 +354 @@
 void PopupListBox::paint(GraphicsContext* gc, const IntRect& rect)
 {
+    int scale = m_settings.defaultDeviceScaleFactor;
     // adjust coords for scrolled frame
     IntRect r = intersection(rect, frameRect());
@@ -367 +369 @@
     // FIXME: Can we optimize scrolling to not require repainting the entire
     // window? Should we?
+    if (scale != 1)
+        gc->scale(FloatSize(scale, scale));
     for (int i = 0; i < numItems(); ++i)
         paintRow(gc, r, i);
@@ -389 +393 @@
     if (!rowRect.intersects(rect))
         return;
+
+    int scale = m_settings.defaultDeviceScaleFactor;
+    // RowRect has already been scaled by the defaultDeviceScaleFactor.
+    // To avoid scaling it twice, we have to unscale it before drawing.
+    if (scale != 1) {
+        // Height and y should both be evenly divisible by scale.
+        ASSERT(!(rowRect.y() % scale));
+        rowRect.setY(rowRect.y() / scale);
+        ASSERT(!(rowRect.height() % scale));
+        rowRect.setHeight(rowRect.height() / scale);
+        rowRect.setWidth(ceilf(static_cast<float>(rowRect.width()) / scale));
+        // rowRect.x is always 0.
+    }
 
     PopupMenuStyle style = m_popupClient->itemStyle(rowIndex);
@@ -614 +631 @@
 int PopupListBox::getRowHeight(int index)
 {
-    if (index < 0)
-        return PopupMenuChromium::minimumRowHeight();
-
-    if (m_popupClient->itemStyle(index).isDisplayNone())
-        return PopupMenuChromium::minimumRowHeight();
+    int scale = m_settings.defaultDeviceScaleFactor;
+    int paddingForTouch = 0;
+    if (RuntimeEnabledFeatures::touchEnabled())
+        paddingForTouch = PopupMenuChromium::optionPaddingForTouch();
+    if (index < 0 || m_popupClient->itemStyle(index).isDisplayNone())
+        return PopupMenuChromium::minimumRowHeight() * scale;
 
     // Separator row height is the same size as itself.
     if (m_popupClient->itemIsSeparator(index))
-        return max(separatorHeight, PopupMenuChromium::minimumRowHeight());
+        return max(separatorHeight, (PopupMenuChromium::minimumRowHeight())) * scale;
 
     String icon = m_popupClient->itemIcon(index);
@@ -632 +650 @@
     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, PopupMenuChromium::minimumRowHeight()) + paddingForTouch) * scale;
 }
 

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

@@ -80 +80 @@
     // Autocomplete popups are restricted, combo-boxes (select tags) aren't.
     bool restrictWidthOfListBox;
+
+    int defaultDeviceScaleFactor;
 };
 

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

@@ -32 +32 @@
 #include "config.h"
 #include "PopupMenuChromium.h"
+
+#include "Frame.h"
+#include "FrameView.h"
+#include "Page.h"
 #include "PopupContainer.h"
+#include "Settings.h"
 
 namespace WebCore {
 
 int PopupMenuChromium::s_minimumRowHeight = 0;
+int PopupMenuChromium::s_optionPaddingForTouch = 30;
 
 // The settings used for the drop down menu.
@@ -63 +69 @@
 void PopupMenuChromium::show(const IntRect& r, FrameView* v, int index)
 {
-    if (!p.popup)
-        p.popup = PopupContainer::create(client(), PopupContainer::Select, dropDownSettings);
+    if (!p.popup) {
+        PopupContainerSettings popupSettings = dropDownSettings;
+        popupSettings.defaultDeviceScaleFactor =
+            v->frame()->page()->settings()->defaultDeviceScaleFactor();
+        if (!popupSettings.defaultDeviceScaleFactor)
+            popupSettings.defaultDeviceScaleFactor = 1;
+        p.popup = PopupContainer::create(client(), PopupContainer::Select, popupSettings);
+    }
     p.popup->showInRect(r, v, index);
 }

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

@@ -58 +58 @@
     static void setMinimumRowHeight(int minimumRowHeight) { s_minimumRowHeight = minimumRowHeight; }
 
+    static int optionPaddingForTouch() { return s_optionPaddingForTouch; }
+    static void setOptionPaddingForTouch(int optionPaddingForTouch) { s_optionPaddingForTouch = optionPaddingForTouch; }
+
 private:
     PopupMenuClient* client() const { return m_popupClient; }
@@ -65 +68 @@
 
     static int s_minimumRowHeight;
+    static int s_optionPaddingForTouch;
 };
 

trunk/Source/WebCore/rendering/RenderMenuList.cpp

@@ -44 +44 @@
 #include "RenderScrollbar.h"
 #include "RenderTheme.h"
+#include "Settings.h"
 #include "TextRun.h"
 #include <math.h>
@@ -309 +310 @@
     FloatPoint absTopLeft = localToAbsolute(FloatPoint(), false, true);
     LayoutRect absBounds = absoluteBoundingBoxRectIgnoringTransforms();
+    int scale = document()->page()->settings()->defaultDeviceScaleFactor();
+    if (scale && scale != 1)
+        absBounds.scale(scale);
     absBounds.setLocation(roundedLayoutPoint(absTopLeft));
     HTMLSelectElement* select = toHTMLSelectElement(node());

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2012-03-09  Tim Dresser  <tdresser@chromium.org>
+
+        [chromium] Increase size of Combo Box Options for touch and high DPI devices
+        https://bugs.webkit.org/show_bug.cgi?id=80027
+
+        Reviewed by Darin Fisher.
+
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::gestureEvent):
+        (WebKit::WebViewImpl::applyAutofillSuggestions):
+
 2012-03-09  James Robinson  <jamesr@chromium.org>
 

trunk/Source/WebKit/chromium/src/WebViewImpl.cpp

@@ -603 +603 @@
 bool WebViewImpl::gestureEvent(const WebGestureEvent& event)
 {
+    RefPtr<WebCore::PopupContainer> selectPopup;
     PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
-    return mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
+    if (event.type == WebInputEvent::GestureTap) {
+        selectPopup = m_selectPopup;
+        hideSelectPopup();
+        ASSERT(!m_selectPopup);
+    }
+
+    bool gestureHandled = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
+
+    if (m_selectPopup && m_selectPopup == selectPopup) {
+        // That tap triggered a select popup which is the same as the one that
+        // was showing before the tap. It means the user tapped the select
+        // while the popup was showing, and as a result we first closed then
+        // immediately reopened the select popup. It needs to be closed.
+        hideSelectPopup();
+    }
+
+    return gestureHandled;
 }
 
@@ -2692 +2709 @@
 
     if (!m_autofillPopup) {
+        PopupContainerSettings popupSettings = autofillPopupSettings;
+        popupSettings.defaultDeviceScaleFactor =
+            m_page->settings()->defaultDeviceScaleFactor();
+        if (!popupSettings.defaultDeviceScaleFactor)
+            popupSettings.defaultDeviceScaleFactor = 1;
         m_autofillPopup = PopupContainer::create(m_autofillPopupClient.get(),
                                                  PopupContainer::Suggestion,
-                                                 autofillPopupSettings);
+                                                 popupSettings);
     }