Attachment 131093 Details for Bug 80027 – Patch

[patch] Patch

bug-80027-20120309164613.patch (text/plain), 12.20 KB, created by Tim Dresser on 2012-03-09 13:46:16 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Dresser

Created: 2012-03-09 13:46:16 PST

Size: 12.20 KB

patch

obsolete

>Subversion Revision: 110319
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 974ac660931a35e0e6a020a1dcfe231bbb50e140..475b993710a6ee4adc3394fcf74d305a21884980 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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  Patrick Gansterer  <paroga@webkit.org>
> 
>         Build fix for !ENABLE(FILTERS) after r110285.
>diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
>index 9155993a516fee8265bc2b2ad0e0a0792d1aff34..fcf2523cda2dbc0d973a84ad11c919ccc655de07 100644
>--- a/Source/WebKit/chromium/ChangeLog
>+++ b/Source/WebKit/chromium/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        * src/WebViewImpl.cpp:
>+        (WebKit::WebViewImpl::gestureEvent):
>+        (WebKit::WebViewImpl::applyAutofillSuggestions):
>+
> 2012-03-09  Dana Jansens  <danakj@chromium.org>
> 
>         [chromium] Use opaque paints in CCOcclusionTracker
>diff --git a/Source/WebCore/platform/chromium/PopupListBox.cpp b/Source/WebCore/platform/chromium/PopupListBox.cpp
>index c980ad623a1f345d91402754f434a6457a4fe257..6d67e05534050bc2650e1df70766207779b8c23e 100644
>--- 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
>--- 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
>--- 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
>--- 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 a47aebbae4401ce4bc57632e0d4fee4918f6f8f5..e982eea6072c5c057bf0bf5b2fde7a59c275b11d 100644
>--- 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/src/WebViewImpl.cpp b/Source/WebKit/chromium/src/WebViewImpl.cpp
>index ec9403cbcd699bf51887f438046dde4a740606af..4d503c705a46736b7cb64ba018a0a24820bed767 100644
>--- a/Source/WebKit/chromium/src/WebViewImpl.cpp
>+++ b/Source/WebKit/chromium/src/WebViewImpl.cpp
>@@ -601,8 +601,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)
>@@ -2690,9 +2707,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) {

Actions: View | Formatted Diff | Diff

Attachments on bug 80027: 129710 | 130614 | 131093 | 131103 | 131104 | 131882 | 133034