Changeset 118707 in webkit

Timestamp:

May 28, 2012 2:55:20 PM (12 years ago)

Author:

commit-queue@webkit.org

Message:

[chromium] Only increase size of Combo Box Options when displayed on touch screen
​https://bugs.webkit.org/show_bug.cgi?id=85921

Patch by Rob Flack <​flackr@chromium.org> on 2012-05-28
Reviewed by Adam Barth.

Adds a flag to set whether the current device is a touch screen, independent of whether touch events are supported and use this for the combo box sizing.

Source/WebCore:

No new tests as this is a flag change and covered by existing tests: WebKit/chromium/tests/PopupMenuTest.cpp

• page/Settings.cpp:

(WebCore::Settings::Settings):

• page/Settings.h:

(WebCore::Settings::setDeviceSupportsTouch):
(WebCore::Settings::deviceSupportsTouch):
(Settings):

• platform/chromium/PopupListBox.cpp:

(WebCore::PopupListBox::getRowHeight):

• platform/chromium/PopupListBox.h:

(PopupContainerSettings):

• platform/chromium/PopupMenuChromium.cpp:

(WebCore::PopupMenuChromium::show):

Source/WebKit/chromium:

• public/WebSettings.h:
• src/WebSettingsImpl.cpp:

(WebKit::WebSettingsImpl::defaultDeviceScaleFactor):
(WebKit):
(WebKit::WebSettingsImpl::setDeviceSupportsTouch):
(WebKit::WebSettingsImpl::deviceSupportsTouch):

• src/WebSettingsImpl.h:

(WebSettingsImpl):

• src/WebViewImpl.cpp:

(WebKit::WebViewImpl::applyAutofillSuggestions):

• tests/PopupMenuTest.cpp:

(WebKit::SelectPopupMenuTest::SetUp):
(WebKit::SelectPopupMenuTest::TearDown):
(SelectPopupMenuTest):
(WebKit::TEST_F):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/Settings.cpp (modified) (1 diff)
• WebCore/page/Settings.h (modified) (2 diffs)
• WebCore/platform/chromium/PopupListBox.cpp (modified) (1 diff)
• WebCore/platform/chromium/PopupListBox.h (modified) (1 diff)
• WebCore/platform/chromium/PopupMenuChromium.cpp (modified) (1 diff)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/public/WebSettings.h (modified) (1 diff)
• WebKit/chromium/src/WebSettingsImpl.cpp (modified) (1 diff)
• WebKit/chromium/src/WebSettingsImpl.h (modified) (1 diff)
• WebKit/chromium/src/WebViewImpl.cpp (modified) (1 diff)
• WebKit/chromium/tests/PopupMenuTest.cpp (modified) (10 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-05-28  Rob Flack  <flackr@chromium.org>
+
+        [chromium] Only increase size of Combo Box Options when displayed on touch screen
+        https://bugs.webkit.org/show_bug.cgi?id=85921
+
+        Reviewed by Adam Barth.
+
+        Adds a flag to set whether the current device is a touch screen, independent of whether touch events are supported and use this for the combo box sizing.
+
+        No new tests as this is a flag change and covered by existing tests: WebKit/chromium/tests/PopupMenuTest.cpp
+
+        * page/Settings.cpp:
+        (WebCore::Settings::Settings):
+        * page/Settings.h:
+        (WebCore::Settings::setDeviceSupportsTouch):
+        (WebCore::Settings::deviceSupportsTouch):
+        (Settings):
+        * platform/chromium/PopupListBox.cpp:
+        (WebCore::PopupListBox::getRowHeight):
+        * platform/chromium/PopupListBox.h:
+        (PopupContainerSettings):
+        * platform/chromium/PopupMenuChromium.cpp:
+        (WebCore::PopupMenuChromium::show):
+
 2012-05-28  Arvid Nilsson  <anilsson@rim.com>
 

trunk/Source/WebCore/page/Settings.cpp

@@ -271 +271 @@
     , m_wantsBalancedSetDefersLoadingBehavior(false)
     , m_requestAnimationFrameEnabled(true)
+    , m_deviceSupportsTouch(false)
     , m_needsDidFinishLoadOrderQuirk(false)
     , m_fixedPositionCreatesStackingContext(false)

trunk/Source/WebCore/page/Settings.h

@@ -575 +575 @@
         void setRequestAnimationFrameEnabled(bool enabled) { m_requestAnimationFrameEnabled = enabled; }
         bool requestAnimationFrameEnabled() const { return m_requestAnimationFrameEnabled; }
+
+        void setDeviceSupportsTouch(bool enabled) { m_deviceSupportsTouch = enabled; }
+        bool deviceSupportsTouch() const { return m_deviceSupportsTouch; }
 
         void setNeedsDidFinishLoadOrderQuirk(bool needsQuirk) { m_needsDidFinishLoadOrderQuirk = needsQuirk; }
@@ -754 +757 @@
         bool m_wantsBalancedSetDefersLoadingBehavior : 1;
         bool m_requestAnimationFrameEnabled : 1;
+        bool m_deviceSupportsTouch : 1;
         bool m_needsDidFinishLoadOrderQuirk : 1;
 

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

@@ -633 +633 @@
     int scale = m_settings.defaultDeviceScaleFactor;
     int paddingForTouch = 0;
-    if (RuntimeEnabledFeatures::touchEnabled())
+    if (m_settings.deviceSupportsTouch)
         paddingForTouch = PopupMenuChromium::optionPaddingForTouch();
     if (index < 0 || m_popupClient->itemStyle(index).isDisplayNone())

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

@@ -81 +81 @@
     bool restrictWidthOfListBox;
 
+    // The default device scale factor of the screen used to draw the menu
+    // at this scale suitable for the device DPI.
     int defaultDeviceScaleFactor;
+
+    // If the device is a touch screen we increase the height of menu items
+    // to make it easier to unambiguously touch them.
+    bool deviceSupportsTouch;
 };
 

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

@@ -67 +67 @@
 }
 
-void PopupMenuChromium::show(const IntRect& r, FrameView* v, int index)
+void PopupMenuChromium::show(const IntRect& rect, FrameView* frameView, int index)
 {
     if (!p.popup) {
+        Settings* settings = frameView->frame()->page()->settings();
         PopupContainerSettings popupSettings = dropDownSettings;
-        popupSettings.defaultDeviceScaleFactor =
-            v->frame()->page()->settings()->defaultDeviceScaleFactor();
+        popupSettings.defaultDeviceScaleFactor = settings->defaultDeviceScaleFactor();
         if (!popupSettings.defaultDeviceScaleFactor)
             popupSettings.defaultDeviceScaleFactor = 1;
+        popupSettings.deviceSupportsTouch = settings->deviceSupportsTouch();
         p.popup = PopupContainer::create(client(), PopupContainer::Select, popupSettings);
     }
-    p.popup->showInRect(r, v, index);
+    p.popup->showInRect(rect, frameView, index);
 }
 

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2012-05-28  Rob Flack  <flackr@chromium.org>
+
+        [chromium] Only increase size of Combo Box Options when displayed on touch screen
+        https://bugs.webkit.org/show_bug.cgi?id=85921
+
+        Reviewed by Adam Barth.
+
+        Adds a flag to set whether the current device is a touch screen, independent of whether touch events are supported and use this for the combo box sizing.
+
+        * public/WebSettings.h:
+        * src/WebSettingsImpl.cpp:
+        (WebKit::WebSettingsImpl::defaultDeviceScaleFactor):
+        (WebKit):
+        (WebKit::WebSettingsImpl::setDeviceSupportsTouch):
+        (WebKit::WebSettingsImpl::deviceSupportsTouch):
+        * src/WebSettingsImpl.h:
+        (WebSettingsImpl):
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::applyAutofillSuggestions):
+        * tests/PopupMenuTest.cpp:
+        (WebKit::SelectPopupMenuTest::SetUp):
+        (WebKit::SelectPopupMenuTest::TearDown):
+        (SelectPopupMenuTest):
+        (WebKit::TEST_F):
+
 2012-05-25  Jesus Sanchez-Palencia  <jesus.palencia@openbossa.org>
 

trunk/Source/WebKit/chromium/public/WebSettings.h

@@ -68 +68 @@
     virtual void setFontBoostingEnabled(bool) = 0;
     virtual void setDefaultTextEncodingName(const WebString&) = 0;
+    virtual void setDeviceSupportsTouch(bool) = 0;
     virtual void setJavaScriptEnabled(bool) = 0;
     virtual void setWebSecurityEnabled(bool) = 0;

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

@@ -119 +119 @@
 }
 
+int WebSettingsImpl::defaultDeviceScaleFactor()
+{
+    return m_settings->defaultDeviceScaleFactor();
+}
+
+void WebSettingsImpl::setDeviceSupportsTouch(bool deviceSupportsTouch)
+{
+    m_settings->setDeviceSupportsTouch(deviceSupportsTouch);
+}
+
+bool WebSettingsImpl::deviceSupportsTouch()
+{
+    return m_settings->deviceSupportsTouch();
+}
+
 void WebSettingsImpl::setApplyDefaultDeviceScaleFactorInCompositor(bool applyDefaultDeviceScaleFactorInCompositor)
 {

trunk/Source/WebKit/chromium/src/WebSettingsImpl.h

@@ -56 +56 @@
     virtual void setMinimumLogicalFontSize(int);
     virtual void setDefaultDeviceScaleFactor(int);
+    virtual int defaultDeviceScaleFactor();
     virtual void setApplyDefaultDeviceScaleFactorInCompositor(bool);
     virtual void setFontBoostingEnabled(bool);
     virtual void setDefaultTextEncodingName(const WebString&);
+    virtual void setDeviceSupportsTouch(bool);
+    virtual bool deviceSupportsTouch();
     virtual void setJavaScriptEnabled(bool);
     virtual void setWebSecurityEnabled(bool);

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

@@ -2956 +2956 @@
     if (!m_autofillPopup) {
         PopupContainerSettings popupSettings = autofillPopupSettings;
-        popupSettings.defaultDeviceScaleFactor =
-            m_page->settings()->defaultDeviceScaleFactor();
+        popupSettings.defaultDeviceScaleFactor = settingsImpl()->defaultDeviceScaleFactor();
         if (!popupSettings.defaultDeviceScaleFactor)
             popupSettings.defaultDeviceScaleFactor = 1;
+        popupSettings.deviceSupportsTouch = settingsImpl()->deviceSupportsTouch();
         m_autofillPopup = PopupContainer::create(m_autofillPopupClient.get(),
                                                  PopupContainer::Suggestion,

trunk/Source/WebKit/chromium/tests/PopupMenuTest.cpp

@@ -183 +183 @@
     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);
@@ -197 +193 @@
         m_webView->close();
         webkit_support::UnregisterAllMockedURLs();
-        RuntimeEnabledFeatures::setTouchEnabled(m_touchWasEnabled);
     }
 
@@ -285 +280 @@
     TestPopupMenuClient m_popupMenuClient;
     RefPtr<PopupMenu> m_popupMenu;
-    bool m_touchWasEnabled;
     std::string baseURL;
 };
@@ -361 +355 @@
     showPopup();
 
-    // Y of 18 to be on the item at index 1 (12 font plus border and more to be safe).
-    IntPoint row1Point(2, 18);
+    int menuItemHeight = m_webView->selectPopup()->menuItemHeight();
+    // menuItemHeight * 1.5 means the Y position on the item at index 1.
+    IntPoint row1Point(2, menuItemHeight * 1.5);
     // Simulate a click down/up on the first item.
     simulateLeftMouseDownEvent(row1Point);
@@ -378 +373 @@
     showPopup();
 
-    // Y of 18 to be on the item at index 1 (12 font plus border and more to be safe).
-    IntPoint row1Point(2, 18);
+    int menuItemHeight = m_webView->selectPopup()->menuItemHeight();
+    // menuItemHeight * 1.5 means the Y position on the item at index 1.
+    IntPoint row1Point(2, menuItemHeight * 1.5);
     // Simulate the mouse moving over the first item.
     PlatformMouseEvent mouseEvent(row1Point, row1Point, NoButton, PlatformEvent::MouseMoved,
@@ -422 +418 @@
     showPopup();
 
-    int menuHeight = m_webView->selectPopup()->menuItemHeight();
-    // menuHeight * 0.5 means the Y position on the item at index 0.
-    IntPoint row1Point(2, menuHeight * 0.5);
+    int menuItemHeight = m_webView->selectPopup()->menuItemHeight();
+    // menuItemHeight * 0.5 means the Y position on the item at index 0.
+    IntPoint row1Point(2, menuItemHeight * 0.5);
     simulateLeftMouseDownEvent(row1Point);
     simulateLeftMouseUpEvent(row1Point);
@@ -438 +434 @@
 
     showPopup();
-    // menuHeight * 1.5 means the Y position on the item at index 1.
-    row1Point.setY(menuHeight * 1.5);
+    // menuItemHeight * 1.5 means the Y position on the item at index 1.
+    row1Point.setY(menuItemHeight * 1.5);
     simulateLeftMouseDownEvent(row1Point);
     simulateLeftMouseUpEvent(row1Point);
@@ -447 +443 @@
 
     showPopup();
-    // menuHeight * 2.5 means the Y position on the item at index 2.
-    row1Point.setY(menuHeight * 2.5);
+    // menuItemHeight * 2.5 means the Y position on the item at index 2.
+    row1Point.setY(menuItemHeight * 2.5);
     simulateLeftMouseDownEvent(row1Point);
     simulateLeftMouseUpEvent(row1Point);
@@ -489 +485 @@
     showPopup();
 
-    int menuHeight = m_webView->selectPopup()->menuItemHeight();
-    // menuHeight * 1.5 means the Y position on the item at index 1.
-    IntPoint row1Point(2, menuHeight * 1.5);
+    int menuItemHeight = m_webView->selectPopup()->menuItemHeight();
+    // menuItemHeight * 1.5 means the Y position on the item at index 1.
+    IntPoint row1Point(2, menuItemHeight * 1.5);
     simulateLeftMouseDownEvent(row1Point);
     simulateLeftMouseUpEvent(row1Point);
@@ -511 +507 @@
     showPopup();
 
-    int menuHeight = m_webView->selectPopup()->menuItemHeight();
-    // menuHeight * 1.5 means the Y position on the item at index 1.
-    IntPoint row1Point(2, menuHeight * 1.5);
+    int menuItemHeight = m_webView->selectPopup()->menuItemHeight();
+    // menuItemHeight * 1.5 means the Y position on the item at index 1.
+    IntPoint row1Point(2, menuItemHeight * 1.5);
     simulateLeftMouseDownEvent(row1Point);
     simulateLeftMouseUpEvent(row1Point);