Page popups can show up at wrong locations
authorkeishi@webkit.org <keishi@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 12:43:21 +0000 (12:43 +0000)
committerkeishi@webkit.org <keishi@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 12:43:21 +0000 (12:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93556

Reviewed by Kent Tamura.

Source/WebCore:

No new tests. Cannot test popup position.

* html/shadow/CalendarPickerElement.cpp:
(WebCore::CalendarPickerElement::contentSize):

Source/WebKit/chromium:

We were showing the popup at wrong positions. When there isn't enough
room below and above the element it adjusts the position but we weren't
resetting the adjustment when we resize the popup. This patch will make
the popup adjust the popup position each time we resize.

* src/ColorChooserUIController.cpp:
(WebKit::ColorChooserUIController::contentSize):
* src/WebPagePopupImpl.cpp:
(WevKit::PagePopupChromeClient::setWindowRect):
(WebKit::WebPagePopupImpl::init): Use reposition().
(WebKit::WebPagePopupImpl::reposition): Repositions the page popup based on the popup size.
(WebKit):
(WebKit::WebPagePopupImpl::resize): Use reposition().
* src/WebPagePopupImpl.h:
(WebPagePopupImpl):

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

Source/WebCore/ChangeLog
Source/WebCore/html/shadow/CalendarPickerElement.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/ColorChooserUIController.cpp
Source/WebKit/chromium/src/WebPagePopupImpl.cpp
Source/WebKit/chromium/src/WebPagePopupImpl.h

index 7083703..e43c2bc 100644 (file)
@@ -1,3 +1,15 @@
+2012-08-09  Keishi Hattori  <keishi@webkit.org>
+
+        Page popups can show up at wrong locations
+        https://bugs.webkit.org/show_bug.cgi?id=93556
+
+        Reviewed by Kent Tamura.
+
+        No new tests. Cannot test popup position.
+
+        * html/shadow/CalendarPickerElement.cpp:
+        (WebCore::CalendarPickerElement::contentSize):
+
 2012-08-08  Andrey Kosyakov  <caseq@chromium.org>
 
         Web Inspector: display progress bar while loading timeline data
index 1feb082..e70151d 100644 (file)
@@ -148,7 +148,7 @@ void CalendarPickerElement::detach()
 
 IntSize CalendarPickerElement::contentSize()
 {
-    return IntSize(100, 100);
+    return IntSize(0, 0);
 }
 
 void CalendarPickerElement::writeDocument(DocumentWriter& writer)
index 456725e..c2b43cb 100644 (file)
@@ -1,3 +1,26 @@
+2012-08-09  Keishi Hattori  <keishi@webkit.org>
+
+        Page popups can show up at wrong locations
+        https://bugs.webkit.org/show_bug.cgi?id=93556
+
+        Reviewed by Kent Tamura.
+
+        We were showing the popup at wrong positions. When there isn't enough
+        room below and above the element it adjusts the position but we weren't
+        resetting the adjustment when we resize the popup. This patch will make
+        the popup adjust the popup position each time we resize.
+
+        * src/ColorChooserUIController.cpp:
+        (WebKit::ColorChooserUIController::contentSize):
+        * src/WebPagePopupImpl.cpp:
+        (WevKit::PagePopupChromeClient::setWindowRect):
+        (WebKit::WebPagePopupImpl::init): Use reposition().
+        (WebKit::WebPagePopupImpl::reposition): Repositions the page popup based on the popup size.
+        (WebKit):
+        (WebKit::WebPagePopupImpl::resize): Use reposition().
+        * src/WebPagePopupImpl.h:
+        (WebPagePopupImpl):
+
 2012-08-09  Peter Beverloo  <peter@chromium.org>
 
         [Chromium] Pull in the android_tools directory for Android
index 08bc124..28cdc38 100644 (file)
@@ -92,7 +92,7 @@ void ColorChooserUIController::didEndChooser()
 
 WebCore::IntSize ColorChooserUIController::contentSize()
 {
-    return WebCore::IntSize(300, 300);
+    return WebCore::IntSize(0, 0);
 }
 
 void ColorChooserUIController::writeDocument(WebCore::DocumentWriter& writer)
index 18a1716..18f5de2 100644 (file)
@@ -78,11 +78,7 @@ private:
 
     virtual void setWindowRect(const FloatRect& rect) OVERRIDE
     {
-        IntRect intRect(rect);
-        const WebRect currentRect = m_popup->m_windowRectInScreen;
-        if (intRect.x() == currentRect.x && intRect.y() == currentRect.y && m_popup->m_isPutAboveOrigin)
-            intRect.setY(currentRect.y + currentRect.height - intRect.height());
-        m_popup->m_windowRectInScreen = intRect;
+        m_popup->m_windowRectInScreen = IntRect(rect);
         m_popup->widgetClient()->setWindowRect(m_popup->m_windowRectInScreen);
     }
 
@@ -146,7 +142,6 @@ bool PagePopupFeaturesClient::isEnabled(Document*, ContextFeatures::FeatureType
 
 WebPagePopupImpl::WebPagePopupImpl(WebWidgetClient* client)
     : m_widgetClient(client)
-    , m_isPutAboveOrigin(false)
 {
     ASSERT(client);
 }
@@ -162,24 +157,13 @@ bool WebPagePopupImpl::init(WebViewImpl* webView, PagePopupClient* popupClient,
     ASSERT(popupClient);
     m_webView = webView;
     m_popupClient = popupClient;
+    m_originBoundsInRootView = originBoundsInRootView;
 
-    WebSize rootViewSize = webView->size();
-    IntSize popupSize = popupClient->contentSize();
-    IntRect popupBoundsInRootView(IntPoint(max(0, originBoundsInRootView.x()), max(0, originBoundsInRootView.maxY())), popupSize);
-    if (popupBoundsInRootView.maxY() > rootViewSize.height) {
-        popupBoundsInRootView.setY(max(0, originBoundsInRootView.y() - popupSize.height()));
-        m_isPutAboveOrigin = true;
-    }
-    if (popupBoundsInRootView.maxX() > rootViewSize.width)
-        popupBoundsInRootView.setX(max(0, rootViewSize.width - popupSize.width()));
-    IntRect boundsInScreen = webView->page()->chrome()->rootViewToScreen(popupBoundsInRootView);
+    reposition(m_popupClient->contentSize());
 
-    m_widgetClient->setWindowRect(boundsInScreen);
-    m_windowRectInScreen = boundsInScreen;
     if (!initPage())
         return false;
     m_widgetClient->show(WebNavigationPolicy());
-
     setFocus(true);
 
     return true;
@@ -247,8 +231,23 @@ void WebPagePopupImpl::paint(WebCanvas* canvas, const WebRect& rect)
     PageWidgetDelegate::paint(m_page.get(), 0, canvas, rect, PageWidgetDelegate::Opaque);
 }
 
+void WebPagePopupImpl::reposition(const WebSize& popupSize)
+{
+    WebSize rootViewSize = m_webView->size();
+    IntRect popupBoundsInRootView(IntPoint(max(0, m_originBoundsInRootView.x()), max(0, m_originBoundsInRootView.maxY())), popupSize);
+    if (popupBoundsInRootView.maxY() > rootViewSize.height)
+        popupBoundsInRootView.setY(max(0, m_originBoundsInRootView.y() - popupSize.height));
+    if (popupBoundsInRootView.maxX() > rootViewSize.width)
+        popupBoundsInRootView.setX(max(0, rootViewSize.width - popupSize.width));
+    IntRect boundsInScreen = m_webView->page()->chrome()->rootViewToScreen(popupBoundsInRootView);
+    m_widgetClient->setWindowRect(boundsInScreen);
+    m_windowRectInScreen = boundsInScreen;
+}
+
 void WebPagePopupImpl::resize(const WebSize& newSize)
 {
+    reposition(newSize);
+
     if (m_page)
         m_page->mainFrame()->view()->resize(newSize);
     m_widgetClient->didInvalidateRect(WebRect(0, 0, newSize.width, newSize.height));
index 2552bac..76936a4 100644 (file)
@@ -87,14 +87,15 @@ private:
 
     explicit WebPagePopupImpl(WebWidgetClient*);
     bool initPage();
+    void reposition(const WebSize&);
 
     WebWidgetClient* m_widgetClient;
     WebRect m_windowRectInScreen;
+    WebCore::IntRect m_originBoundsInRootView;
     WebViewImpl* m_webView;
     OwnPtr<WebCore::Page> m_page;
     OwnPtr<PagePopupChromeClient> m_chromeClient;
     WebCore::PagePopupClient* m_popupClient;
-    bool m_isPutAboveOrigin;
 
     friend class WebPagePopup;
     friend class PagePopupChromeClient;