[Mac] Should be able to scroll embedded PDFs using mouse
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Oct 2011 23:52:21 +0000 (23:52 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Oct 2011 23:52:21 +0000 (23:52 +0000)
        https://bugs.webkit.org/show_bug.cgi?id=69579

        Reviewed by Darin Adler.

        Hit testing doesn't find scrollbars installed by a plug-in, so mouse events don't work.

        As we want to use WebKit scrollbars with PDF plug-in, a cleaner long-term solution would be
        to have a base class under PluginViewBase that can handle scrolling instead of doing that
        in PluginView and Plugin.

        * plugins/PluginViewBase.h:
        (WebCore::PluginViewBase::horizontalScrollbar):
        (WebCore::PluginViewBase::verticalScrollbar):
        Added accessors for scrollbars installed by plug-in.

        * rendering/nodeAtPoint.cpp: (WebCore::RenderEmbeddedObject::nodeAtPoint):
        Like RenderLayer, add scrollbars to hit result after normal  hit testing.

        * rendering/RenderEmbeddedObject.h: Added a nodeAtPoint override.

        * rendering/RenderWidget.h: Made nodeAtPoint() protected instead of private, so that
        RenderEmbeddedObject could call it.

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

15 files changed:
Source/WebCore/ChangeLog
Source/WebCore/plugins/PluginViewBase.h
Source/WebCore/rendering/RenderEmbeddedObject.cpp
Source/WebCore/rendering/RenderEmbeddedObject.h
Source/WebCore/rendering/RenderWidget.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp
Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.h
Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.cpp
Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.h
Source/WebKit2/WebProcess/Plugins/Plugin.h
Source/WebKit2/WebProcess/Plugins/PluginProxy.cpp
Source/WebKit2/WebProcess/Plugins/PluginProxy.h
Source/WebKit2/WebProcess/Plugins/PluginView.cpp
Source/WebKit2/WebProcess/Plugins/PluginView.h

index a0b4b78..610c875 100644 (file)
@@ -1,3 +1,29 @@
+2011-10-06  Alexey Proskuryakov  <ap@apple.com>
+
+        [Mac] Should be able to scroll embedded PDFs using mouse
+        https://bugs.webkit.org/show_bug.cgi?id=69579
+
+        Reviewed by Darin Adler.
+
+        Hit testing doesn't find scrollbars installed by a plug-in, so mouse events don't work.
+
+        As we want to use WebKit scrollbars with PDF plug-in, a cleaner long-term solution would be
+        to have a base class under PluginViewBase that can handle scrolling instead of doing that
+        in PluginView and Plugin.
+
+        * plugins/PluginViewBase.h:
+        (WebCore::PluginViewBase::horizontalScrollbar):
+        (WebCore::PluginViewBase::verticalScrollbar):
+        Added accessors for scrollbars installed by plug-in.
+
+        * rendering/nodeAtPoint.cpp: (WebCore::RenderEmbeddedObject::nodeAtPoint):
+        Like RenderLayer, add scrollbars to hit result after normal  hit testing.
+
+        * rendering/RenderEmbeddedObject.h: Added a nodeAtPoint override.
+
+        * rendering/RenderWidget.h: Made nodeAtPoint() protected instead of private, so that
+        RenderEmbeddedObject could call it.
+
 2011-10-06  Dan Bernstein  <mitz@apple.com>
 
         <rdar://problem/9717490> Flash of white when navigating daringfireball.net
index 77cce91..fa839d4 100644 (file)
@@ -38,6 +38,8 @@ namespace JSC {
 
 namespace WebCore {
 
+class Scrollbar;
+
 // PluginViewBase is a widget that all plug-in views inherit from, both in Webkit and WebKit2.
 // It's intended as a stopgap measure until we can merge all plug-in views into a single plug-in view.
 class PluginViewBase : public Widget {
@@ -51,6 +53,10 @@ public:
     virtual bool getFormValue(String&) { return false; }
     virtual bool scroll(ScrollDirection, ScrollGranularity) { return false; }
 
+    // A plug-in can ask WebKit to handle scrollbars for it.
+    virtual Scrollbar* horizontalScrollbar() { return 0; }
+    virtual Scrollbar* verticalScrollbar() { return 0; }
+
 protected:
     PluginViewBase(PlatformWidget widget = 0) : Widget(widget) { }
     
index 6c5e128..5425bc6 100644 (file)
@@ -38,6 +38,7 @@
 #include "HTMLNames.h"
 #include "HTMLObjectElement.h"
 #include "HTMLParamElement.h"
+#include "HitTestResult.h"
 #include "LocalizedStrings.h"
 #include "MIMETypeRegistry.h"
 #include "MouseEvent.h"
@@ -250,6 +251,33 @@ void RenderEmbeddedObject::viewCleared()
     }
 }
 
+bool RenderEmbeddedObject::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, const LayoutPoint& pointInContainer, const LayoutPoint& accumulatedOffset, HitTestAction hitTestAction)
+{
+    if (!RenderPart::nodeAtPoint(request, result, pointInContainer, accumulatedOffset, hitTestAction))
+        return false;
+
+    if (!widget() || !widget()->isPluginViewBase())
+        return true;
+
+    PluginViewBase* view = static_cast<PluginViewBase*>(widget());
+
+    if (Scrollbar* horizontalScrollbar = view->horizontalScrollbar()) {
+        if (horizontalScrollbar->frameRect().contains(pointInContainer)) {
+            result.setScrollbar(horizontalScrollbar);
+            return true;
+        }
+    }
+
+    if (Scrollbar* verticalScrollbar = view->verticalScrollbar()) {
+        if (verticalScrollbar->frameRect().contains(pointInContainer)) {
+            result.setScrollbar(verticalScrollbar);
+            return true;
+        }
+    }
+
+    return true;
+}
+
 bool RenderEmbeddedObject::scroll(ScrollDirection direction, ScrollGranularity granularity, float, Node**)
 {
     if (!widget() || !widget()->isPluginViewBase())
index 5bf31d4..c579160 100644 (file)
@@ -67,6 +67,8 @@ private:
     virtual void layout();
     virtual void viewCleared();
 
+    virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const LayoutPoint& pointInContainer, const LayoutPoint& accumulatedOffset, HitTestAction);
+
     virtual bool scroll(ScrollDirection, ScrollGranularity, float multiplier, Node** stopNode);
     virtual bool logicalScroll(ScrollLogicalDirection, ScrollGranularity, float multiplier, Node** stopNode);
 
index 04bd639..7051039 100644 (file)
@@ -60,6 +60,7 @@ protected:
     virtual void layout();
     virtual void paint(PaintInfo&, const LayoutPoint&);
     virtual CursorDirective getCursor(const LayoutPoint&, Cursor&) const;
+    virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const LayoutPoint& pointInContainer, const LayoutPoint& accumulatedOffset, HitTestAction);
 
 private:
     virtual bool isWidget() const { return true; }
@@ -67,7 +68,6 @@ private:
     virtual void willBeDestroyed();
     virtual void destroy();
     virtual void setSelectionState(SelectionState);
-    virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const LayoutPoint& pointInContainer, const LayoutPoint& accumulatedOffset, HitTestAction);
     virtual void setOverlapTestResult(bool);
 
     bool setWidgetGeometry(const IntRect&);
index c9b1ceb..0e336f2 100644 (file)
@@ -1,3 +1,35 @@
+2011-10-06  Alexey Proskuryakov  <ap@apple.com>
+
+        [Mac] Should be able to scroll embedded PDFs using mouse
+        https://bugs.webkit.org/show_bug.cgi?id=69579
+
+        Reviewed by Darin Adler.
+
+        * WebProcess/Plugins/PDF/BuiltInPDFView.cpp:
+        (WebKit::BuiltInPDFView::handleMouseEvent): No longer need to hardcode mouseDown handling,
+        ScrollableArea does everything now.
+        (WebKit::BuiltInPDFView::horizontalScrollbar): Added plumbing to access scrollbars from
+        WebCore hit testing code.
+        (WebKit::BuiltInPDFView::verticalScrollbar): Ditto.
+        (WebKit::BuiltInPDFView::visibleHeight): ScrollableArea now wants this.
+        (WebKit::BuiltInPDFView::visibleWidth): Ditto.
+
+        * WebProcess/Plugins/Netscape/NetscapePlugin.cpp:
+        (WebKit::NetscapePlugin::horizontalScrollbar):
+        (WebKit::NetscapePlugin::verticalScrollbar):
+        * WebProcess/Plugins/Netscape/NetscapePlugin.h:
+        * WebProcess/Plugins/PDF/BuiltInPDFView.h:
+        * WebProcess/Plugins/Plugin.h:
+        * WebProcess/Plugins/PluginProxy.cpp:
+        (WebKit::PluginProxy::horizontalScrollbar):
+        (WebKit::PluginProxy::verticalScrollbar):
+        * WebProcess/Plugins/PluginProxy.h:
+        * WebProcess/Plugins/PluginView.cpp:
+        (WebKit::PluginView::horizontalScrollbar):
+        (WebKit::PluginView::verticalScrollbar):
+        * WebProcess/Plugins/PluginView.h:
+        Added plumbing to access scrollbars from WebCore hit testing code.
+
 2011-10-06  Alexis Menard  <alexis.menard@openbossa.org>
 
         [Qt][WK2] WebIconDatabase expects a file path not a directory.
index 7cbc792..d4fce8b 100644 (file)
@@ -885,6 +885,16 @@ bool NetscapePlugin::handleScroll(ScrollDirection, ScrollGranularity)
     return false;
 }
 
+Scrollbar* NetscapePlugin::horizontalScrollbar()
+{
+    return 0;
+}
+
+Scrollbar* NetscapePlugin::verticalScrollbar()
+{
+    return 0;
+}
+
 bool NetscapePlugin::supportsSnapshotting() const
 {
 #if PLATFORM(MAC)
index e543913..2a8e53c 100644 (file)
@@ -206,6 +206,8 @@ private:
     virtual void privateBrowsingStateChanged(bool);
     virtual bool getFormValue(String& formValue);
     virtual bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity);
+    virtual WebCore::Scrollbar* horizontalScrollbar();
+    virtual WebCore::Scrollbar* verticalScrollbar();
 
     bool supportsSnapshotting() const;
 
index 14f6a4b..e02ab6f 100644 (file)
@@ -380,13 +380,9 @@ bool BuiltInPDFView::handleMouseEvent(const WebMouseEvent& event)
         // FIXME: Should also notify scrollbar to show hover effect. Should also send mouseExited to hide it.
         break;
     case WebEvent::MouseDown: {
-        PlatformMouseEvent mouseDownEvent = platform(event);
-        if (m_horizontalScrollbar)
-            m_horizontalScrollbar->mouseDown(mouseDownEvent);
-        if (m_verticalScrollbar)
-            m_verticalScrollbar->mouseDown(mouseDownEvent);
-        // Returning false as that will make EventHandler unfocus the plug-in, which is appropriate when clicking scrollbars.
-        // When support for PDF forms is added, we'll need to actually focus the plug-in.
+        // Returning false as will make EventHandler unfocus the plug-in, which is appropriate when clicking scrollbars.
+        // Ideally, we wouldn't change focus at all, but PluginView already did that for us.
+        // When support for PDF forms is added, we'll need to actually focus the plug-in when clicking in a form.
         break;
     }
     case WebEvent::MouseUp:
@@ -473,6 +469,16 @@ bool BuiltInPDFView::handleScroll(ScrollDirection direction, ScrollGranularity g
     return scroll(direction, granularity);
 }
 
+Scrollbar* BuiltInPDFView::horizontalScrollbar()
+{
+    return m_horizontalScrollbar.get();
+}
+
+Scrollbar* BuiltInPDFView::verticalScrollbar()
+{
+    return m_verticalScrollbar.get();
+}
+
 IntRect BuiltInPDFView::scrollCornerRect() const
 {
     if (!m_horizontalScrollbar || !m_verticalScrollbar)
@@ -556,6 +562,16 @@ IntPoint BuiltInPDFView::maximumScrollPosition() const
     return maximumOffset;
 }
 
+LayoutUnit BuiltInPDFView::visibleHeight() const
+{
+    return m_frameRect.height();
+}
+
+LayoutUnit BuiltInPDFView::visibleWidth() const
+{
+    return m_frameRect.width();
+}
+
 IntSize BuiltInPDFView::contentsSize() const
 {
     return m_pdfDocumentSize;
index 11ace11..1b3c942 100644 (file)
@@ -103,6 +103,8 @@ private:
     virtual void privateBrowsingStateChanged(bool);
     virtual bool getFormValue(String& formValue);
     virtual bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity);
+    virtual WebCore::Scrollbar* horizontalScrollbar();
+    virtual WebCore::Scrollbar* verticalScrollbar();
 
     // ScrollableArea methods.
     virtual WebCore::IntRect scrollCornerRect() const;
@@ -117,6 +119,8 @@ private:
     virtual WebCore::IntPoint scrollPosition() const;
     virtual WebCore::IntPoint minimumScrollPosition() const;
     virtual WebCore::IntPoint maximumScrollPosition() const;
+    virtual WebCore::LayoutUnit visibleHeight() const;
+    virtual WebCore::LayoutUnit visibleWidth() const;
     virtual WebCore::IntSize contentsSize() const;
     virtual WebCore::Scrollbar* horizontalScrollbar() const  { return m_horizontalScrollbar.get(); }
     virtual WebCore::Scrollbar* verticalScrollbar() const { return m_verticalScrollbar.get(); }
index ec2fcbd..ebdd1b7 100644 (file)
@@ -42,6 +42,7 @@ namespace CoreIPC {
 namespace WebCore {
     class GraphicsContext;
     class IntRect;
+    class Scrollbar;
 }
 
 namespace WebKit {
@@ -203,6 +204,11 @@ public:
     // Tells the plug-in that it should scroll. The plug-in should return true if it did scroll.
     virtual bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity) = 0;
 
+    // A plug-in can use WebCore scroll bars. Make them known, so that hit testing can find them.
+    // FIXME: This code should be in PluginView or its base class, not in individual plug-ins.
+    virtual WebCore::Scrollbar* horizontalScrollbar() = 0;
+    virtual WebCore::Scrollbar* verticalScrollbar() = 0;
+
 protected:
     Plugin();
 
index 1e674c4..204f411 100644 (file)
@@ -391,6 +391,16 @@ bool PluginProxy::handleScroll(ScrollDirection, ScrollGranularity)
     return false;
 }
 
+Scrollbar* PluginProxy::horizontalScrollbar()
+{
+    return 0;
+}
+
+Scrollbar* PluginProxy::verticalScrollbar()
+{
+    return 0;
+}
+
 void PluginProxy::loadURL(uint64_t requestID, const String& method, const String& urlString, const String& target, const HTTPHeaderMap& headerFields, const Vector<uint8_t>& httpBody, bool allowPopups)
 {
     controller()->loadURL(requestID, method, urlString, target, headerFields, httpBody, allowPopups);
index 51b3717..41c16ce 100644 (file)
@@ -103,6 +103,8 @@ private:
     virtual void privateBrowsingStateChanged(bool);
     virtual bool getFormValue(String& formValue);
     virtual bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity);
+    virtual WebCore::Scrollbar* horizontalScrollbar();
+    virtual WebCore::Scrollbar* verticalScrollbar();
 
     bool needsBackingStore() const;
     uint64_t windowNPObjectID();
index 64e078b..04f32fb 100644 (file)
@@ -528,6 +528,24 @@ bool PluginView::scroll(ScrollDirection direction, ScrollGranularity granularity
     return m_plugin->handleScroll(direction, granularity);
 }
 
+Scrollbar* PluginView::horizontalScrollbar()
+{
+    // The plug-in can be null here if it failed to initialize.
+    if (!m_isInitialized || !m_plugin)
+        return 0;
+
+    return m_plugin->horizontalScrollbar();
+}
+
+Scrollbar* PluginView::verticalScrollbar()
+{
+    // The plug-in can be null here if it failed to initialize.
+    if (!m_isInitialized || !m_plugin)
+        return 0;
+
+    return m_plugin->verticalScrollbar();
+}
+
 void PluginView::setFrameRect(const WebCore::IntRect& rect)
 {
     Widget::setFrameRect(rect);
index a23c963..ce196b8 100644 (file)
@@ -104,6 +104,8 @@ private:
     virtual void privateBrowsingStateChanged(bool);
     virtual bool getFormValue(String&);
     virtual bool scroll(WebCore::ScrollDirection, WebCore::ScrollGranularity);
+    virtual WebCore::Scrollbar* horizontalScrollbar();
+    virtual WebCore::Scrollbar* verticalScrollbar();
 
     // WebCore::Widget
     virtual void setFrameRect(const WebCore::IntRect&);