[chromium] Increase size of Combo Box Options for touch and high DPI devices
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 10 Mar 2012 02:40:26 +0000 (02:40 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 10 Mar 2012 02:40:26 +0000 (02:40 +0000)
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):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/chromium/PopupListBox.cpp
Source/WebCore/platform/chromium/PopupListBox.h
Source/WebCore/platform/chromium/PopupMenuChromium.cpp
Source/WebCore/platform/chromium/PopupMenuChromium.h
Source/WebCore/rendering/RenderMenuList.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebViewImpl.cpp

index ca6c070..7330513 100644 (file)
@@ -1,3 +1,33 @@
+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>
 
         Improve ContainerNode collectNode() performance by reserving vector
index c980ad6..6d67e05 100644 (file)
@@ -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)
index dd2e58d..b685313 100644 (file)
@@ -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>
index 3b2f2ae..62e1008 100644 (file)
 
 #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);
 }
 
index 20be6e1..6e9aa88 100644 (file)
@@ -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
index a47aebb..e982eea 100644 (file)
@@ -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()));
index 99f4ae3..d1b5dd8 100644 (file)
@@ -1,3 +1,14 @@
+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>
 
         [chromium] Roll chromium DEPS to r125600
index 5a9c909..db8362c 100644 (file)
@@ -602,8 +602,25 @@ bool WebViewImpl::mouseWheel(const WebMouseWheelEvent& event)
 #if ENABLE(GESTURE_EVENTS)
 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;
 }
 
 void WebViewImpl::startPageScaleAnimation(const IntPoint& scroll, bool useAnchor, float newScale, double durationSec)
@@ -2691,9 +2708,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) {