[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-21
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):
* tests/PopupMenuTest.cpp:
(WebKit::TestWebViewClient::screenInfo):
(WebKit::SelectPopupMenuTest::SetUp):
(WebKit::SelectPopupMenuTest::TearDown):
(SelectPopupMenuTest):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 0d715a16c8ef6f2f44f1359fa978be574b71390f..9ed5552264cd366281048c1ba107af8b2a7fe635 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2012-03-21  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-21  Takashi Toyoshima  <toyoshim@chromium.org>
 
         [Chromium] [WebSocket] provide WebFrameClient with a chance of

diff --git a/Source/WebCore/platform/chromium/PopupListBox.cpp b/Source/WebCore/platform/chromium/PopupListBox.cpp
index c980ad623a1f345d91402754f434a6457a4fe257..6d67e05534050bc2650e1df70766207779b8c23e 100644 (file)
--- a/Source/WebCore/platform/chromium/PopupListBox.cpp
+++ b/Source/WebCore/platform/chromium/PopupListBox.cpp
@@ -44,6 +44,7 @@
 #include "PopupMenuChromium.h"
 #include "PopupMenuClient.h"
 #include "RenderTheme.h"
+#include "RuntimeEnabledFeatures.h"
 #include "ScrollbarTheme.h"
 #include "StringTruncator.h"
 #include "TextRun.h"
@@ -352,6 +353,7 @@ void PopupListBox::typeAheadFind(const PlatformKeyboardEvent& event)
 
 void PopupListBox::paint(GraphicsContext* gc, const IntRect& rect)
 {
+    int scale = m_settings.defaultDeviceScaleFactor;
     // adjust coords for scrolled frame
     IntRect r = intersection(rect, frameRect());
     int tx = x() - scrollX();
@@ -366,6 +368,8 @@ void PopupListBox::paint(GraphicsContext* gc, const IntRect& rect)
 
     // 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,6 +393,19 @@ void PopupListBox::paintRow(GraphicsContext* gc, const IntRect& rect, int rowInd
     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);
 
     // Paint background
@@ -613,15 +630,16 @@ void PopupListBox::setOriginalIndex(int index)
 
 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);
     RefPtr<Image> image(Image::loadPlatformResource(icon.utf8().data()));
@@ -631,7 +649,7 @@ int PopupListBox::getRowHeight(int index)
 
     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;
 }
 
 IntRect PopupListBox::getRowBounds(int index)

diff --git a/Source/WebCore/platform/chromium/PopupListBox.h b/Source/WebCore/platform/chromium/PopupListBox.h
index dd2e58deddd8f38d2463e623af79038cc523f879..b685313cc4319b114642f75e0797529d685bc1a3 100644 (file)
--- a/Source/WebCore/platform/chromium/PopupListBox.h
+++ b/Source/WebCore/platform/chromium/PopupListBox.h
@@ -79,6 +79,8 @@ struct PopupContainerSettings {
     // Whether we should restrict the width of the PopupListBox or not.
     // Autocomplete popups are restricted, combo-boxes (select tags) aren't.
     bool restrictWidthOfListBox;
+
+    int defaultDeviceScaleFactor;
 };
 
 // A container for the data for each menu item (e.g. represented by <option>

diff --git a/Source/WebCore/platform/chromium/PopupMenuChromium.cpp b/Source/WebCore/platform/chromium/PopupMenuChromium.cpp
index 3b2f2ae9fe34328abcedb3d95a19f18e51294c20..62e1008fb96a00ce5c42871f4d8c4015d66a70c1 100644 (file)
--- a/Source/WebCore/platform/chromium/PopupMenuChromium.cpp
+++ b/Source/WebCore/platform/chromium/PopupMenuChromium.cpp
@@ -31,11 +31,17 @@
 
 #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.
 // This is the delegate used if none is provided.
@@ -62,8 +68,14 @@ PopupMenuChromium::~PopupMenuChromium()
 
 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);
 }
 

diff --git a/Source/WebCore/platform/chromium/PopupMenuChromium.h b/Source/WebCore/platform/chromium/PopupMenuChromium.h
index 20be6e153cbed7cacb3516e190149727155e4f01..6e9aa88be5e385e0c8e848042a04d488eb1d7822 100644 (file)
--- a/Source/WebCore/platform/chromium/PopupMenuChromium.h
+++ b/Source/WebCore/platform/chromium/PopupMenuChromium.h
@@ -57,6 +57,9 @@ public:
     static int minimumRowHeight() { return s_minimumRowHeight; }
     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; }
 
@@ -64,6 +67,7 @@ private:
     PopupMenuPrivate p;
 
     static int s_minimumRowHeight;
+    static int s_optionPaddingForTouch;
 };
 
 } // namespace WebCore

diff --git a/Source/WebCore/rendering/RenderMenuList.cpp b/Source/WebCore/rendering/RenderMenuList.cpp
index 70659382d4ca2bbcdd35d969a7ffb3ab9bcf7bdc..17debf81812c84cce99af06e4093effc4803106f 100644 (file)
--- a/Source/WebCore/rendering/RenderMenuList.cpp
+++ b/Source/WebCore/rendering/RenderMenuList.cpp
@@ -43,6 +43,7 @@
 #include "RenderBR.h"
 #include "RenderScrollbar.h"
 #include "RenderTheme.h"
+#include "Settings.h"
 #include "TextRun.h"
 #include <math.h>
 
@@ -308,6 +309,9 @@ void RenderMenuList::showPopup()
     // the actual width of the element to size the popup.
     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());
     m_popup->show(absBounds, document()->view(), select->optionToListIndex(select->selectedIndex()));

diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
index 8a7f4eb7a33a17582884fac8c66c9cd1431721ee..6934922fab565725501a2dacf216aeec5df96dea 100644 (file)
--- a/Source/WebKit/chromium/ChangeLog
+++ b/Source/WebKit/chromium/ChangeLog
@@ -1,3 +1,19 @@
+2012-03-21  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):
+        * tests/PopupMenuTest.cpp:
+        (WebKit::TestWebViewClient::screenInfo):
+        (WebKit::SelectPopupMenuTest::SetUp):
+        (WebKit::SelectPopupMenuTest::TearDown):
+        (SelectPopupMenuTest):
+
 2012-03-21  Takashi Toyoshima  <toyoshim@chromium.org>
 
         [Chromium] [WebSocket] provide WebFrameClient with a chance of

diff --git a/Source/WebKit/chromium/src/WebViewImpl.cpp b/Source/WebKit/chromium/src/WebViewImpl.cpp
index 11aeeba7519d7a1b9a86e827ca7b6cac3ae56f87..a81266fdebac64a4b7dced87cb5ff07d90b170dc 100644 (file)
--- a/Source/WebKit/chromium/src/WebViewImpl.cpp
+++ b/Source/WebKit/chromium/src/WebViewImpl.cpp
@@ -644,10 +644,25 @@ bool WebViewImpl::gestureEvent(const WebGestureEvent& event)
             return true;
         }
         return false;
+    case WebInputEvent::GestureTap: {
+        PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
+        RefPtr<WebCore::PopupContainer> selectPopup;
+        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;
+    }
     case WebInputEvent::GestureScrollBegin:
     case WebInputEvent::GestureScrollEnd:
     case WebInputEvent::GestureScrollUpdate:
-    case WebInputEvent::GestureTap:
     case WebInputEvent::GestureTapDown:
     case WebInputEvent::GestureDoubleTap:
     case WebInputEvent::GesturePinchBegin:
@@ -2753,9 +2768,14 @@ void WebViewImpl::applyAutofillSuggestions(
         inputElem, names, labels, icons, uniqueIDs, separatorIndex);
 
     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);
     }
 
     if (m_autofillPopupShowing) {

diff --git a/Source/WebKit/chromium/tests/PopupMenuTest.cpp b/Source/WebKit/chromium/tests/PopupMenuTest.cpp
index f053ca410ee742add9573495dbd89e76c3e95f51..1db65ec6ef2706680bc6e3db311a889ea2abfea1 100644 (file)
--- a/Source/WebKit/chromium/tests/PopupMenuTest.cpp
+++ b/Source/WebKit/chromium/tests/PopupMenuTest.cpp
@@ -42,6 +42,7 @@
 #include "PopupMenu.h"
 #include "PopupMenuClient.h"
 #include "PopupMenuChromium.h"
+#include "RuntimeEnabledFeatures.h"
 #include "WebDocument.h"
 #include "WebElement.h"
 #include "WebFrame.h"
@@ -154,7 +155,7 @@ public:
     // We need to override this so that the popup menu size is not 0
     // (the layout code checks to see if the popup fits on the screen).
     virtual WebScreenInfo screenInfo()
-    { 
+    {
         WebScreenInfo screenInfo;
         screenInfo.availableRect.height = 2000;
         screenInfo.availableRect.width = 2000;
@@ -181,6 +182,10 @@ public:
 protected:
     virtual void SetUp()
     {
+        // When touch is enabled, padding is added to option elements
+        // In these tests, we'll assume touch is disabled.
+        m_touchWasEnabled = RuntimeEnabledFeatures::touchEnabled();
+        RuntimeEnabledFeatures::setTouchEnabled(false);
         m_webView = static_cast<WebViewImpl*>(WebView::create(&m_webviewClient));
         m_webView->initializeMainFrame(&m_webFrameClient);
         m_popupMenu = adoptRef(new PopupMenuChromium(&m_popupMenuClient));
@@ -191,6 +196,7 @@ protected:
         m_popupMenu = 0;
         m_webView->close();
         webkit_support::UnregisterAllMockedURLs();
+        RuntimeEnabledFeatures::setTouchEnabled(m_touchWasEnabled);
     }
 
     // Returns true if there currently is a select popup in the WebView.
@@ -278,6 +284,7 @@ protected:
     TestWebFrameClient m_webFrameClient;
     TestPopupMenuClient m_popupMenuClient;
     RefPtr<PopupMenu> m_popupMenu;
+    bool m_touchWasEnabled;
     std::string baseURL;
 };