2011-01-17 Naoki Takano <takano.naoki@gmail.com>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Jan 2011 00:44:20 +0000 (00:44 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Jan 2011 00:44:20 +0000 (00:44 +0000)
        Reviewed by Kent Tamura.

        [Chromium] Fix popup menu re-positioning when the menu is opened upward, above the corresponding form field.
        https://bugs.webkit.org/show_bug.cgi?id=51382
        http://crbug.com/60427

        Calculate correct location of popup window whenever the items in the window change.

        No new tests, because this fix is for Chromium project and hard to test only in WebKit project

        * platform/chromium/PopupMenuChromium.cpp:
        (WebCore::PopupContainer::layoutAndCalculateWidgetRect): New Function to layout and calculate popup widget rect.
        (WebCore::PopupContainer::showPopup): Move widgetRect calculation logic to calculateWidgetRect().
        (WebCore::PopupContainer::refresh): Add parameter focusRect to take the location and the size of focus text input field to calculate correct popup window location.
        * platform/chromium/PopupMenuChromium.h: Append new input parameter for refresh().
2011-01-17  Naoki Takano  <takano.naoki@gmail.com>

        Reviewed by Kent Tamura.

        [Chromium] Fix popup menu re-positioning when the menu is opened upward, above the corresponding form field.
        https://bugs.webkit.org/show_bug.cgi?id=51382
        http://crbug.com/60427

        Calculate correct location of popup window whenever the items in the window change.

        No new tests, because this fix is for Chromium project and hard to test only in WebKit project

       * WebKit/chromium/src/WebViewImpl.cpp:
       (WebKit::WebViewImpl::refreshAutoFillPopup): Change the logic in refreshAutoFilPopup() to check both the location and the size of popup window.

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

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

index 1c3aef89d8c9615b26ab8df1a470ef11775c9aa9..3302905d77e9a576d051bff101818fb8dc90de1b 100644 (file)
@@ -1,3 +1,21 @@
+2011-01-17  Naoki Takano  <takano.naoki@gmail.com>
+
+        Reviewed by Kent Tamura.
+
+        [Chromium] Fix popup menu re-positioning when the menu is opened upward, above the corresponding form field.
+        https://bugs.webkit.org/show_bug.cgi?id=51382
+        http://crbug.com/60427
+
+        Calculate correct location of popup window whenever the items in the window change.
+
+        No new tests, because this fix is for Chromium project and hard to test only in WebKit project
+
+        * platform/chromium/PopupMenuChromium.cpp:
+        (WebCore::PopupContainer::layoutAndCalculateWidgetRect): New Function to layout and calculate popup widget rect.
+        (WebCore::PopupContainer::showPopup): Move widgetRect calculation logic to calculateWidgetRect().
+        (WebCore::PopupContainer::refresh): Add parameter focusRect to take the location and the size of focus text input field to calculate correct popup window location.
+        * platform/chromium/PopupMenuChromium.h: Append new input parameter for refresh().
+
 2011-01-17  Mark Rowe  <mrowe@apple.com>
 
         Fix the 32-bit build.
index 04eeb9305a395721981fb3731133a602618a924c..bb45e7949451f02a32694557aa5847ac1add3f6b 100644 (file)
@@ -329,11 +329,8 @@ PopupContainer::~PopupContainer()
         removeChild(m_listBox.get());
 }
 
-void PopupContainer::showPopup(FrameView* view)
+IntRect PopupContainer::layoutAndCalculateWidgetRect(int targetControlHeight, const IntPoint& popupInitialCoordinate)
 {
-    // Pre-layout, our size matches the <select> dropdown control.
-    int selectHeight = frameRect().height();
-
     // Reset the max height to its default value, it will be recomputed below
     // if necessary.
     m_listBox->setMaxHeight(kMaxHeight);
@@ -341,23 +338,26 @@ void PopupContainer::showPopup(FrameView* view)
     // Lay everything out to figure out our preferred size, then tell the view's
     // WidgetClient about it.  It should assign us a client.
     layout();
+  
+    // Assume m_listBox size is already calculated.
+    IntSize targetSize(m_listBox->width() + kBorderSize * 2,
+                       m_listBox->height() + kBorderSize * 2);
 
-    m_frameView = view;
+    IntRect widgetRect;
     ChromeClientChromium* chromeClient = chromeClientChromium();
     if (chromeClient) {
         // If the popup would extend past the bottom of the screen, open upwards
         // instead.
-        FloatRect screen = screenAvailableRect(view);
-        IntRect widgetRect = chromeClient->windowToScreen(frameRect());
-
+        FloatRect screen = screenAvailableRect(m_frameView.get());
+        widgetRect = chromeClient->windowToScreen(IntRect(popupInitialCoordinate, targetSize));
         if (widgetRect.bottom() > static_cast<int>(screen.bottom())) {
-            if (widgetRect.y() - widgetRect.height() - selectHeight > 0) {
+            if (widgetRect.y() - widgetRect.height() - targetControlHeight > 0) {
                 // There is enough room to open upwards.
-                widgetRect.move(0, -(widgetRect.height() + selectHeight));
+                widgetRect.move(0, -(widgetRect.height() + targetControlHeight));
             } else {
                 // Figure whether upwards or downwards has more room and set the
                 // maximum number of items.
-                int spaceAbove = widgetRect.y() - selectHeight;
+                int spaceAbove = widgetRect.y() - targetControlHeight;
                 int spaceBelow = screen.bottom() - widgetRect.y();
                 if (spaceAbove > spaceBelow)
                     m_listBox->setMaxHeight(spaceAbove);
@@ -368,10 +368,21 @@ void PopupContainer::showPopup(FrameView* view)
                 widgetRect = chromeClient->windowToScreen(frameRect());
                 // And move upwards if necessary.
                 if (spaceAbove > spaceBelow)
-                    widgetRect.move(0, -(widgetRect.height() + selectHeight));
+                    widgetRect.move(0, -(widgetRect.height() + targetControlHeight));
             }
         }
-        chromeClient->popupOpened(this, widgetRect, false);
+    }
+    return widgetRect;
+}
+
+void PopupContainer::showPopup(FrameView* view)
+{
+    m_frameView = view;
+
+    ChromeClientChromium* chromeClient = chromeClientChromium();
+    if (chromeClient) {
+        IntRect popupRect = frameRect();
+        chromeClient->popupOpened(this, layoutAndCalculateWidgetRect(popupRect.height(), popupRect.location()), false);
         m_popupOpen = true;
     }
 
@@ -556,15 +567,24 @@ void PopupContainer::show(const IntRect& r, FrameView* v, int index)
     // Move it below the select widget.
     location.move(0, r.height());
 
-    IntRect popupRect(location, r.size());
-    setFrameRect(popupRect);
+    setFrameRect(IntRect(location, r.size()));
     showPopup(v);
 }
 
-void PopupContainer::refresh()
+void PopupContainer::refresh(const IntRect& targetControlRect)
 {
+    IntPoint location = m_frameView->contentsToWindow(targetControlRect.location());
+    // Move it below the select widget.
+    location.move(0, targetControlRect.height());
+
     listBox()->updateFromElement();
-    layout();
+    // Store the original height to check if we need to request the location.
+    int originalHeight = height();
+    IntRect widgetRect = layoutAndCalculateWidgetRect(targetControlRect.height(), location);
+    if (originalHeight != widgetRect.height())
+        setFrameRect(widgetRect);
+
+    invalidate();
 }
 
 int PopupContainer::selectedIndex() const
index ca47ccfb252dfac158ea02bb78a2f861206baa4d..f326b48273d163a4a66a159229e14212c3ee8ab6 100644 (file)
@@ -168,7 +168,7 @@ public:
     int selectedIndex() const;
 
     // Refresh the popup values from the PopupMenuClient.
-    void refresh();
+    void refresh(const IntRect& targetControlRect);
 
     // The menu per-item data.
     const WTF::Vector<PopupItem*>& popupData() const;
@@ -193,6 +193,9 @@ private:
     // Paint the border.
     void paintBorder(GraphicsContext*, const IntRect&);
 
+    // Layout and calculate popup widget size and location and returns it as IntRect.
+    IntRect layoutAndCalculateWidgetRect(int targetControlHeight, const IntPoint& popupInitialCoordinate);
+
     // Returns the ChromeClient of the page this popup is associated with.
     ChromeClientChromium* chromeClientChromium();
 
index 52d5b464bb4f501b3e41bc65b19765693254cee8..efada7bd4f540cc6390b22decfe0ac7225d64a59 100644 (file)
@@ -1,3 +1,18 @@
+2011-01-17  Naoki Takano  <takano.naoki@gmail.com>
+
+        Reviewed by Kent Tamura.
+
+        [Chromium] Fix popup menu re-positioning when the menu is opened upward, above the corresponding form field.
+        https://bugs.webkit.org/show_bug.cgi?id=51382
+        http://crbug.com/60427
+
+        Calculate correct location of popup window whenever the items in the window change.
+
+        No new tests, because this fix is for Chromium project and hard to test only in WebKit project
+
+       * WebKit/chromium/src/WebViewImpl.cpp:
+       (WebKit::WebViewImpl::refreshAutoFillPopup): Change the logic in refreshAutoFilPopup() to check both the location and the size of popup window.
+
 2011-01-17  Pavel Feldman  <pfeldman@chromium.org>
 
         Reviewed by Yury Semikhatsky.
index bcd97c49d633f8fe263df351acc070271ffe071d..41a08046e348eb39089f437fda4823cbfd9eb9b0 100644 (file)
@@ -2197,9 +2197,9 @@ void WebViewImpl::refreshAutoFillPopup()
         return;
     }
 
-    IntRect oldBounds = m_autoFillPopup->boundsRect();
-    m_autoFillPopup->refresh();
-    IntRect newBounds = m_autoFillPopup->boundsRect();
+    IntRect oldBounds = m_autoFillPopup->frameRect();
+    m_autoFillPopup->refresh(focusedWebCoreNode()->getRect());
+    IntRect newBounds = m_autoFillPopup->frameRect();
     // Let's resize the backing window if necessary.
     if (oldBounds != newBounds) {
         WebPopupMenuImpl* popupMenu =