Towards making PopupMenuClient more testable
authorfsamuel@chromium.org <fsamuel@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Oct 2011 00:19:29 +0000 (00:19 +0000)
committerfsamuel@chromium.org <fsamuel@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Oct 2011 00:19:29 +0000 (00:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=69631

Reviewed by Simon Fraser.

Source/WebCore:

Added some functions to window.internals to allow testing of parts of PopupMenuClient.

Test: fast/dom/popup-menu-client-test.html

* platform/PopupMenuClient.h:
* rendering/RenderMenuList.cpp:
(WebCore::RenderMenuList::showPopup):
(WebCore::RenderMenuList::boundingBoxRect):
* rendering/RenderMenuList.h:
(WebCore::RenderMenuList::RenderMenuList::isPopupMenuClient):
* rendering/RenderObject.h:
(WebCore::RenderObject::isPopupMenuClient):
* rendering/RenderTextControlSingleLine.cpp:
(WebCore::RenderTextControlSingleLine::showPopup):
(WebCore::RenderTextControlSingleLine::boundingBoxRect):
* rendering/RenderTextControlSingleLine.h:
(WebCore::RenderTextControlSingleLine::isPopupMenuClient):
* testing/Internals.cpp:
(WebCore::Internals::toPopupMenuClient):
(WebCore::Internals::popupClientPaddingLeft):
(WebCore::Internals::popupClientPaddingRight):
(WebCore::Internals::popupClientBoundingBoxRect):
* testing/Internals.h:
* testing/Internals.idl:

Source/WebKit/chromium:

* src/AutofillPopupMenuClient.cpp:
(WebKit::AutofillPopupMenuClient::boundingBoxRect):
* src/AutofillPopupMenuClient.h:
* src/WebViewImpl.cpp:
(WebKit::WebViewImpl::applyAutofillSuggestions):

LayoutTests:

* fast/dom/popup-menu-client-test-expected.txt: Added.
* fast/dom/popup-menu-client-test.html: Added.

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/popup-menu-client-test-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/popup-menu-client-test.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/PopupMenuClient.h
Source/WebCore/rendering/RenderMenuList.cpp
Source/WebCore/rendering/RenderMenuList.h
Source/WebCore/rendering/RenderObject.h
Source/WebCore/rendering/RenderTextControlSingleLine.cpp
Source/WebCore/rendering/RenderTextControlSingleLine.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp
Source/WebKit/chromium/src/AutofillPopupMenuClient.h
Source/WebKit/chromium/src/WebViewImpl.cpp

index 5e61e2a443ea86d872dd00e9fa8ee9ed9c8dba29..b4e873c20ae507afdad78aa012537bda7a8ab30f 100644 (file)
@@ -1,3 +1,13 @@
+2011-10-11  Fady Samuel  <fsamuel@chromium.org>
+
+        Towards making PopupMenuClient more testable
+        https://bugs.webkit.org/show_bug.cgi?id=69631
+
+        Reviewed by Simon Fraser.
+
+        * fast/dom/popup-menu-client-test-expected.txt: Added.
+        * fast/dom/popup-menu-client-test.html: Added.
+
 2011-10-11  Kenneth Russell  <kbr@google.com>
 
         [chromium] Check for lost context at beginning of compositor's execution
diff --git a/LayoutTests/fast/dom/popup-menu-client-test-expected.txt b/LayoutTests/fast/dom/popup-menu-client-test-expected.txt
new file mode 100644 (file)
index 0000000..f83e891
--- /dev/null
@@ -0,0 +1,10 @@
+PASS popupClientPaddingLeft is 4
+PASS popupClientPaddingRight is 2
+FAIL boundingBoxRect.left should be 10. Was 20.
+FAIL boundingBoxRect.top should be 10. Was 20.
+PASS boundingBoxRect.width is boundingClientRect.width
+PASS boundingBoxRect.height is boundingClientRect.height
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/popup-menu-client-test.html b/LayoutTests/fast/dom/popup-menu-client-test.html
new file mode 100644 (file)
index 0000000..4737617
--- /dev/null
@@ -0,0 +1,38 @@
+<html>
+  <head>
+    <body>
+      <div id="console"></div>
+      <select id="selectelement">
+        <option>Element 1</option>
+        <option>Element 2</option>
+        <option>Element 3</option>
+      </select>
+    </body>
+    <script src="../js/resources/js-test-pre.js"></script>
+    <script>
+      var selectElement = document.getElementById("selectelement");
+      if (window.internals) {
+        // FIXME: This wil fail if the page scale factor != 1.0
+        var pageScaleFactor = 2.0;
+        eventSender.scalePageBy(pageScaleFactor, 0, 0);
+
+        var popupClientPaddingLeft = window.internals.popupClientPaddingLeft(selectElement);
+        var popupClientPaddingRight = window.internals.popupClientPaddingRight(selectElement);
+        var boundingBoxRect = window.internals.popupClientBoundingBoxRect(selectElement);
+        var boundingClientRect = selectElement.getBoundingClientRect();
+        shouldBe("popupClientPaddingLeft", "4");
+        shouldBe("popupClientPaddingRight", "2");
+        shouldBe("boundingBoxRect.left", "boundingClientRect.left");
+        shouldBe("boundingBoxRect.top", "boundingClientRect.top");
+        shouldBe("boundingBoxRect.width", "boundingClientRect.width");
+        shouldBe("boundingBoxRect.height", "boundingClientRect.height");
+      }
+      if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+      }
+      successfullyParsed = true;
+    </script>
+    <script src="../js/resources/js-test-post.js"></script>
+    </script>
+  </head>
+</html>
index 9a7b1aedf2f176515174f8f8cbb35c42e202276e..8a917d098313b89b32a743fce3f73afdcbf2b468 100644 (file)
@@ -1,3 +1,35 @@
+2011-10-11  Fady Samuel  <fsamuel@chromium.org>
+
+        Towards making PopupMenuClient more testable
+        https://bugs.webkit.org/show_bug.cgi?id=69631
+
+        Reviewed by Simon Fraser.
+
+        Added some functions to window.internals to allow testing of parts of PopupMenuClient.
+
+        Test: fast/dom/popup-menu-client-test.html
+
+        * platform/PopupMenuClient.h:
+        * rendering/RenderMenuList.cpp:
+        (WebCore::RenderMenuList::showPopup):
+        (WebCore::RenderMenuList::boundingBoxRect):
+        * rendering/RenderMenuList.h:
+        (WebCore::RenderMenuList::RenderMenuList::isPopupMenuClient):
+        * rendering/RenderObject.h:
+        (WebCore::RenderObject::isPopupMenuClient):
+        * rendering/RenderTextControlSingleLine.cpp:
+        (WebCore::RenderTextControlSingleLine::showPopup):
+        (WebCore::RenderTextControlSingleLine::boundingBoxRect):
+        * rendering/RenderTextControlSingleLine.h:
+        (WebCore::RenderTextControlSingleLine::isPopupMenuClient):
+        * testing/Internals.cpp:
+        (WebCore::Internals::toPopupMenuClient):
+        (WebCore::Internals::popupClientPaddingLeft):
+        (WebCore::Internals::popupClientPaddingRight):
+        (WebCore::Internals::popupClientBoundingBoxRect):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2011-10-11  Nate Chapin  <japhet@chromium.org>
 
         Make CachedResourceClientWalker templates, and
index f33b9e21b891054d543e27186ef4827590277b67..59aea1594fbaf69169bf36c007e2b2b4276caa28 100644 (file)
@@ -22,6 +22,7 @@
 #ifndef PopupMenuClient_h
 #define PopupMenuClient_h
 
+#include "LayoutTypes.h"
 #include "PopupMenuStyle.h"
 #include "ScrollTypes.h"
 #include <wtf/Forward.h>
@@ -49,6 +50,7 @@ public:
     virtual bool itemIsEnabled(unsigned listIndex) const = 0;
     virtual PopupMenuStyle itemStyle(unsigned listIndex) const = 0;
     virtual PopupMenuStyle menuStyle() const = 0;
+    virtual LayoutRect boundingBoxRect() const = 0;
     virtual int clientInsetLeft() const = 0;
     virtual int clientInsetRight() const = 0;
     virtual int clientPaddingLeft() const = 0;
index 57a3ad24845fe492ef0b3387aa9cbd39d74bf59e..3d48d36b012d4a4e99975ff3fc73d5899ab99a05 100644 (file)
@@ -303,13 +303,7 @@ void RenderMenuList::showPopup()
     HTMLSelectElement* select = toSelectElement(static_cast<Element*>(node()));
     m_popupIsVisible = true;
 
-    // Compute the top left taking transforms into account, but use
-    // the actual width of the element to size the popup.
-    FloatPoint absTopLeft = localToAbsolute(FloatPoint(), false, true);
-    LayoutRect absBounds = absoluteBoundingBoxRectIgnoringTransforms();
-    absBounds.setLocation(roundedIntPoint(absTopLeft));
-    m_popup->show(absBounds, document()->view(),
-        select->optionToListIndex(select->selectedIndex()));
+    m_popup->show(boundingBoxRect(), document()->view(), select->optionToListIndex(select->selectedIndex()));
 }
 
 void RenderMenuList::hidePopup()
@@ -506,6 +500,16 @@ PassRefPtr<Scrollbar> RenderMenuList::createScrollbar(ScrollableArea* scrollable
     return widget.release();
 }
 
+LayoutRect RenderMenuList::boundingBoxRect() const
+{
+    // Compute the top left taking transforms into account, but use
+    // the actual width of the element to size the popup.
+    FloatPoint absTopLeft = localToAbsolute(FloatPoint(), false, true);
+    LayoutRect absBounds = absoluteBoundingBoxRectIgnoringTransforms();
+    absBounds.setLocation(roundedIntPoint(absTopLeft));
+    return absBounds;
+}
+
 int RenderMenuList::clientInsetLeft() const
 {
     return 0;
index 73f981b1997855a89b55cebc38650ef03f26aa4d..e005c7c0428c2dbe06c1fde7211fed6f896fd344 100644 (file)
@@ -39,9 +39,9 @@ namespace WebCore {
 class RenderText;
 
 #if ENABLE(NO_LISTBOX_RENDERING)
-class RenderMenuList : public RenderDeprecatedFlexibleBox, private ListPopupMenuClient {
+class RenderMenuList : public RenderDeprecatedFlexibleBox, public ListPopupMenuClient {
 #else
-class RenderMenuList : public RenderDeprecatedFlexibleBox, private PopupMenuClient {
+class RenderMenuList : public RenderDeprecatedFlexibleBox, public PopupMenuClient {
 #endif
 
 public:
@@ -61,6 +61,7 @@ public:
 
 private:
     virtual bool isMenuList() const { return true; }
+    virtual bool isPopupMenuClient() const { return true; }
 
     virtual void addChild(RenderObject* newChild, RenderObject* beforeChild = 0);
     virtual void removeChild(RenderObject*);
@@ -89,6 +90,7 @@ private:
     virtual bool itemIsEnabled(unsigned listIndex) const;
     virtual PopupMenuStyle itemStyle(unsigned listIndex) const;
     virtual PopupMenuStyle menuStyle() const;
+    virtual LayoutRect boundingBoxRect() const;
     virtual int clientInsetLeft() const;
     virtual int clientInsetRight() const;
     virtual int clientPaddingLeft() const;
index 05a23731d480a49d2b23c3a232c9865366b20066..e1b2dad40d0939aef5bdbab6f76f9f793e2b10a5 100644 (file)
@@ -52,6 +52,7 @@ class InlineBox;
 class InlineFlowBox;
 class OverlapTestRequestClient;
 class Path;
+class PopupMenuClient;
 class Position;
 class RenderBoxModelObject;
 class RenderInline;
@@ -296,6 +297,7 @@ public:
 #if ENABLE(METER_TAG)
     virtual bool isMeter() const { return false; }
 #endif
+    virtual bool isPopupMenuClient() const { return false; }
 #if ENABLE(PROGRESS_TAG)
     virtual bool isProgress() const { return false; }
 #endif
index ff90770f09557891075db91e29e1dfe78704afb6..55a4621b85a54fc7bb9da9586abfe5112efd3988 100644 (file)
@@ -174,7 +174,7 @@ void RenderTextControlSingleLine::showPopup()
         m_searchPopup->saveRecentSearches(name, m_recentSearches);
     }
 
-    m_searchPopup->popupMenu()->show(absoluteBoundingBoxRect(), document()->view(), -1);
+    m_searchPopup->popupMenu()->show(boundingBoxRect(), document()->view(), -1);
 }
 
 void RenderTextControlSingleLine::hidePopup()
@@ -613,6 +613,11 @@ PopupMenuStyle RenderTextControlSingleLine::menuStyle() const
     return PopupMenuStyle(style()->visitedDependentColor(CSSPropertyColor), style()->visitedDependentColor(CSSPropertyBackgroundColor), style()->font(), style()->visibility() == VISIBLE, style()->display() == NONE, style()->textIndent(), style()->direction(), style()->unicodeBidi() == Override);
 }
 
+LayoutRect RenderTextControlSingleLine::boundingBoxRect() const
+{
+    return absoluteBoundingBoxRect();
+}
+
 int RenderTextControlSingleLine::clientInsetLeft() const
 {
     // Inset the menu by the radius of the cap on the left so that
index 474d353659b495e2c8fe6b274e324dc0b1744c0a..f17f713e4d848b1832696ccfdecfde1f46168d42 100644 (file)
@@ -56,6 +56,7 @@ public:
 private:
     virtual bool hasControlClip() const;
     virtual LayoutRect controlClipRect(const LayoutPoint&) const;
+    virtual bool isPopupMenuClient() const { return true; }
     virtual bool isTextField() const { return true; }
 
     virtual void paint(PaintInfo&, const LayoutPoint&);
@@ -100,6 +101,7 @@ private:
     virtual bool itemIsEnabled(unsigned listIndex) const;
     virtual PopupMenuStyle itemStyle(unsigned listIndex) const;
     virtual PopupMenuStyle menuStyle() const;
+    virtual LayoutRect boundingBoxRect() const;
     virtual int clientInsetLeft() const;
     virtual int clientInsetRight() const;
     virtual int clientPaddingLeft() const;
index e149364600439b57c0465871eb971c74f59d0654..65cdd5cb825605ac7595fe67f89f3727d3d3ec71 100644 (file)
@@ -36,6 +36,7 @@
 #include "FrameView.h"
 #include "HTMLInputElement.h"
 #include "HTMLNames.h"
+#include "HTMLSelectElement.h"
 #include "HTMLTextAreaElement.h"
 #include "InspectorController.h"
 #include "IntRect.h"
@@ -44,7 +45,9 @@
 #if ENABLE(GESTURE_EVENTS)
 #include "PlatformGestureEvent.h"
 #endif
+#include "PopupMenuClient.h"
 #include "Range.h"
+#include "RenderMenuList.h"
 #include "RenderObject.h"
 #include "RenderTreeAsText.h"
 #if ENABLE(SMOOTH_SCROLLING)
@@ -435,4 +438,62 @@ void Internals::scrollElementToRect(Element* element, long x, long y, long w, lo
     frameView->scrollElementToRect(element, IntRect(x, y, w, h));
 }
 
+static const PopupMenuClient* toPopupMenuClient(RenderObject* object)
+{
+    ASSERT(!object || object->isPopupMenuClient());
+    if (!object)
+        return 0;
+
+    // Special case: RenderMenuList uses multiple inheritance and so,
+    // depending on the compiler, we may end up in a situation where
+    // the first vptr does not point to the PopupMenuClient's vtable,
+    // and so we'd end up calling the wrong method. Thus, we first cast to
+    // RenderMenuList and then to PopupMenuClient which will automagically
+    // take into account thunking if necessary (the casting will actually shift
+    // the pointer over to the PopupMenuClient part of the object).
+    if (object->isMenuList())
+        return static_cast<PopupMenuClient*>(toRenderMenuList(object));
+
+    return reinterpret_cast<PopupMenuClient*>(object);
+}
+
+int Internals::popupClientPaddingLeft(Element* element, ExceptionCode& ec)
+{
+    if (!element || !element->renderer()) {
+      ec = INVALID_ACCESS_ERR;
+      return 0;
+    }
+    element->document()->updateLayoutIgnorePendingStylesheets();
+    RenderObject* renderer = element->renderer();
+    if (renderer->isPopupMenuClient())
+        return toPopupMenuClient(renderer)->clientPaddingLeft();
+    return 0;
+}
+
+int Internals::popupClientPaddingRight(Element* element, ExceptionCode& ec)
+{
+    if (!element || !element->renderer()) {
+      ec = INVALID_ACCESS_ERR;
+      return 0;
+    }
+    element->document()->updateLayoutIgnorePendingStylesheets();
+    RenderObject* renderer = element->renderer();
+    if (renderer->isPopupMenuClient())
+        return toPopupMenuClient(renderer)->clientPaddingRight();
+    return 0;
+}
+
+PassRefPtr<ClientRect> Internals::popupClientBoundingBoxRect(Element* element, ExceptionCode& ec)
+{
+    if (!element || !element->renderer()) {
+        ec = INVALID_ACCESS_ERR;
+        return ClientRect::create();
+    }
+    element->document()->updateLayoutIgnorePendingStylesheets();
+    RenderObject* renderer = element->renderer();
+    if (renderer->isPopupMenuClient())
+        return ClientRect::create(toPopupMenuClient(renderer)->boundingBoxRect());
+    return ClientRect::create();
+}
+
 }
index 410b6ed8d7840485f34cace85e95718f1996edc3..a3dc93df80e7fabc82fd20e443f75091cd5987e0 100644 (file)
@@ -38,7 +38,9 @@ class ClientRect;
 class Document;
 class Element;
 class Node;
+class PopupMenuClient;
 class Range;
+class RenderObject;
 
 class Internals : public RefCounted<Internals> {
 public:
@@ -92,6 +94,10 @@ public:
     void setSuggestedValue(Element* inputElement, const String&, ExceptionCode&);
     void scrollElementToRect(Element*, long x, long y, long w, long h, ExceptionCode&);
 
+    int popupClientPaddingLeft(Element*, ExceptionCode&);
+    int popupClientPaddingRight(Element*, ExceptionCode&);
+    PassRefPtr<ClientRect> popupClientBoundingBoxRect(Element*, ExceptionCode&);
+
     static const char* internalsId;
 
     void paintControlTints(Document*, ExceptionCode&);
index 909989da2a2a2d42c7f9fca522c0785375c1775f..7cce34540559d35b4574296cfeb93ae38ffe6039 100644 (file)
@@ -67,6 +67,10 @@ module window {
         void paintControlTints(in Document document) raises (DOMException);
 
         void scrollElementToRect(in Element element, in long x, in long y, in long w, in long h) raises (DOMException);
+
+        int popupClientPaddingLeft(in Element element) raises (DOMException);
+        int popupClientPaddingRight(in Element element) raises (DOMException);
+        ClientRect popupClientBoundingBoxRect(in Element element) raises (DOMException);
     };
 }
 
index f6b8b2c13627a028dd7c2dd90216c52f0fdd1d50..021fa4adf3ffb8beb36fffede9264e1093360bce 100644 (file)
@@ -1,3 +1,16 @@
+2011-10-11  Fady Samuel  <fsamuel@chromium.org>
+
+        Towards making PopupMenuClient more testable
+        https://bugs.webkit.org/show_bug.cgi?id=69631
+
+        Reviewed by Simon Fraser.
+
+        * src/AutofillPopupMenuClient.cpp:
+        (WebKit::AutofillPopupMenuClient::boundingBoxRect):
+        * src/AutofillPopupMenuClient.h:
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::applyAutofillSuggestions):
+
 2011-10-11  Dominic Mazzoni  <dmazzoni@google.com>
 
         WebAccessibilityObject needs titleUIElement
index 34a184f1897fdbe1e77c458a36ebaa773edb1754..f77b68a1620c0d453aa60d5d55701aad92985f38 100644 (file)
@@ -216,6 +216,13 @@ int AutofillPopupMenuClient::clientPaddingRight() const
     return RenderTheme::defaultTheme()->popupInternalPaddingRight(style);
 }
 
+WebCore::LayoutRect AutofillPopupMenuClient::boundingBoxRect() const
+{
+    if (m_textField)
+        return m_textField->getRect();
+    return WebCore::LayoutRect();
+}
+
 void AutofillPopupMenuClient::popupDidHide()
 {
     WebViewImpl* webView = getWebView();
index 7d2b985a2c2cce3d089974c0cb58e6268d58c7b3..8e2ec83877c4451639e679ed22e5a0cf34632416 100644 (file)
@@ -31,6 +31,7 @@
 #ifndef AutofillPopupMenuClient_h
 #define AutofillPopupMenuClient_h
 
+#include "LayoutTypes.h"
 #include "PopupMenuClient.h"
 
 namespace WebCore {
@@ -82,6 +83,7 @@ public:
     virtual bool itemIsEnabled(unsigned listIndex) const;
     virtual WebCore::PopupMenuStyle itemStyle(unsigned listIndex) const;
     virtual WebCore::PopupMenuStyle menuStyle() const;
+    virtual WebCore::LayoutRect boundingBoxRect() const;
     virtual int clientInsetLeft() const { return 0; }
     virtual int clientInsetRight() const { return 0; }
     virtual int clientPaddingLeft() const;
index d2518ed0d0f8ac84510fcb611f71349b6faa85ed..e71b32d932410b9fa5ea1c286f8d60e8c94fb23c 100644 (file)
@@ -2241,7 +2241,7 @@ void WebViewImpl::applyAutofillSuggestions(
     if (m_autofillPopupShowing) {
         refreshAutofillPopup();
     } else {
-        m_autofillPopup->showInRect(focusedNode->getRect(), focusedNode->ownerDocument()->view(), 0);
+        m_autofillPopup->showInRect(m_autofillPopupClient->boundingBoxRect(), focusedNode->ownerDocument()->view(), 0);
         m_autofillPopupShowing = true;
     }
 }