From 7cb185627a88d6ad76fde33365fa9273ee7c92bd Mon Sep 17 00:00:00 2001 From: "hyatt@apple.com" Date: Mon, 22 Sep 2008 07:57:52 +0000 Subject: [PATCH] 2008-09-22 David Hyatt Fix a hit testing bug where events are mistakenly passed to subframes if the mouse is over the border or padding area of the frame. Add a boolean flag, isOverWidget(), to hit test results so that EventHandler can check it to tell if the mouse is really over the content box of a RenderWidget and not just in the border/padding area. This is not testable, since the old code properly recovered when it detected that the mouse was outside the bounds of the view, but this prevents the extra passdown from even occurring (and is basically a nice cleanup). Reviewed by Oliver Hunt * page/EventHandler.cpp: (WebCore::EventHandler::handleMousePressEvent): (WebCore::EventHandler::hitTestResultAtPoint): (WebCore::subframeForHitTestResult): (WebCore::EventHandler::handleMouseDoubleClickEvent): (WebCore::EventHandler::handleMouseMoveEvent): (WebCore::EventHandler::handleMouseReleaseEvent): (WebCore::EventHandler::handleWheelEvent): * page/MouseEventWithHitTestResults.h: (WebCore::MouseEventWithHitTestResults::isOverWidget): * rendering/HitTestResult.cpp: (WebCore::HitTestResult::HitTestResult): (WebCore::HitTestResult::operator=): * rendering/HitTestResult.h: (WebCore::HitTestResult::isOverWidget): (WebCore::HitTestResult::setIsOverWidget): * rendering/RenderWidget.cpp: (WebCore::RenderWidget::nodeAtPoint): * rendering/RenderWidget.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@36759 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebCore/ChangeLog | 34 +++++++++++++++++++++ WebCore/page/EventHandler.cpp | 28 ++++++++++------- WebCore/page/MouseEventWithHitTestResults.h | 1 + WebCore/rendering/HitTestResult.cpp | 3 ++ WebCore/rendering/HitTestResult.h | 3 ++ WebCore/rendering/RenderWidget.cpp | 12 ++++++++ WebCore/rendering/RenderWidget.h | 2 ++ 7 files changed, 72 insertions(+), 11 deletions(-) diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index e65bb8289c6..d18e15b4aa4 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,3 +1,37 @@ +2008-09-22 David Hyatt + + Fix a hit testing bug where events are mistakenly passed to subframes + if the mouse is over the border or padding area of the frame. Add + a boolean flag, isOverWidget(), to hit test results so that EventHandler + can check it to tell if the mouse is really over the content box of a + RenderWidget and not just in the border/padding area. + + This is not testable, since the old code properly recovered when it detected + that the mouse was outside the bounds of the view, but this prevents + the extra passdown from even occurring (and is basically a nice cleanup). + + Reviewed by Oliver Hunt + + * page/EventHandler.cpp: + (WebCore::EventHandler::handleMousePressEvent): + (WebCore::EventHandler::hitTestResultAtPoint): + (WebCore::subframeForHitTestResult): + (WebCore::EventHandler::handleMouseDoubleClickEvent): + (WebCore::EventHandler::handleMouseMoveEvent): + (WebCore::EventHandler::handleMouseReleaseEvent): + (WebCore::EventHandler::handleWheelEvent): + * page/MouseEventWithHitTestResults.h: + (WebCore::MouseEventWithHitTestResults::isOverWidget): + * rendering/HitTestResult.cpp: + (WebCore::HitTestResult::HitTestResult): + (WebCore::HitTestResult::operator=): + * rendering/HitTestResult.h: + (WebCore::HitTestResult::isOverWidget): + (WebCore::HitTestResult::setIsOverWidget): + * rendering/RenderWidget.cpp: + (WebCore::RenderWidget::nodeAtPoint): + * rendering/RenderWidget.h: + 2008-09-21 David Hyatt Rename FrameView's repaintRectangle method to repaintContentRectangle. Make diff --git a/WebCore/page/EventHandler.cpp b/WebCore/page/EventHandler.cpp index 9ef49de7a22..29809e8e0fc 100644 --- a/WebCore/page/EventHandler.cpp +++ b/WebCore/page/EventHandler.cpp @@ -93,7 +93,8 @@ using namespace SVGNames; // When the autoscroll or the panScroll is triggered when do the scroll every 0.05s to make it smooth const double autoscrollInterval = 0.05; -static Frame* subframeForTargetNode(Node* node); +static Frame* subframeForTargetNode(Node*); +static Frame* subframeForHitTestResult(const MouseEventWithHitTestResults&); static inline void scrollAndAcceptEvent(float delta, ScrollDirection positiveDirection, ScrollDirection negativeDirection, bool pageScrollEnabled, PlatformWheelEvent& e, Node* node, float windowHeightOrWidth) { @@ -330,7 +331,7 @@ bool EventHandler::handleMousePressEvent(const MouseEventWithHitTestResults& eve m_mouseDownWasSingleClickInSelection = false; - if (passWidgetMouseDownEventToWidget(event)) + if (event.isOverWidget() && passWidgetMouseDownEventToWidget(event)) return true; #if ENABLE(SVG) @@ -687,7 +688,7 @@ HitTestResult EventHandler::hitTestResultAtPoint(const IntPoint& point, bool all while (true) { Node* n = result.innerNode(); - if (!n || !n->renderer() || !n->renderer()->isWidget()) + if (!result.isOverWidget() || !n || !n->renderer() || !n->renderer()->isWidget()) break; Widget* widget = static_cast(n->renderer())->widget(); if (!widget || !widget->isFrameView()) @@ -790,6 +791,13 @@ IntPoint EventHandler::currentMousePosition() const return m_currentMousePosition; } +Frame* subframeForHitTestResult(const MouseEventWithHitTestResults& hitTestResult) +{ + if (!hitTestResult.isOverWidget()) + return 0; + return subframeForTargetNode(hitTestResult.targetNode()); +} + Frame* subframeForTargetNode(Node* node) { if (!node) @@ -999,7 +1007,7 @@ bool EventHandler::handleMousePressEvent(const PlatformMouseEvent& mouseEvent) return true; } - Frame* subframe = subframeForTargetNode(mev.targetNode()); + Frame* subframe = subframeForHitTestResult(mev); if (subframe && passMousePressEventToSubframe(mev, subframe)) { // Start capturing future events for this frame. We only do this if we didn't clear // the m_mousePressed flag, which may happen if an AppKit widget entered a modal event loop. @@ -1097,7 +1105,7 @@ bool EventHandler::handleMouseDoubleClickEvent(const PlatformMouseEvent& mouseEv m_currentMousePosition = mouseEvent.pos(); MouseEventWithHitTestResults mev = prepareMouseEvent(HitTestRequest(false, true), mouseEvent); - Frame* subframe = subframeForTargetNode(mev.targetNode()); + Frame* subframe = subframeForHitTestResult(mev); if (subframe && passMousePressEventToSubframe(mev, subframe)) { m_capturingMouseEventsNode = 0; return true; @@ -1194,9 +1202,8 @@ bool EventHandler::handleMouseMoveEvent(const PlatformMouseEvent& mouseEvent, Hi } bool swallowEvent = false; - Node* targetNode = m_capturingMouseEventsNode ? m_capturingMouseEventsNode.get() : mev.targetNode(); - RefPtr newSubframe = subframeForTargetNode(targetNode); - + RefPtr newSubframe = m_capturingMouseEventsNode.get() ? subframeForTargetNode(m_capturingMouseEventsNode.get()) : subframeForHitTestResult(mev); + // We want mouseouts to happen first, from the inside out. First send a move event to the last subframe so that it will fire mouseouts. if (m_lastMouseMoveEventSubframe && m_lastMouseMoveEventSubframe->tree()->isDescendantOf(m_frame) && m_lastMouseMoveEventSubframe != newSubframe) passMouseMoveEventToSubframe(mev, m_lastMouseMoveEventSubframe.get()); @@ -1261,8 +1268,7 @@ bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent) } MouseEventWithHitTestResults mev = prepareMouseEvent(HitTestRequest(false, false, false, true), mouseEvent); - Node* targetNode = m_capturingMouseEventsNode.get() ? m_capturingMouseEventsNode.get() : mev.targetNode(); - Frame* subframe = subframeForTargetNode(targetNode); + Frame* subframe = m_capturingMouseEventsNode.get() ? subframeForTargetNode(m_capturingMouseEventsNode.get()) : subframeForHitTestResult(mev); if (subframe && passMouseReleaseEventToSubframe(mev, subframe)) { m_capturingMouseEventsNode = 0; return true; @@ -1516,7 +1522,7 @@ bool EventHandler::handleWheelEvent(PlatformWheelEvent& e) // Figure out which view to send the event to. RenderObject* target = node->renderer(); - if (target && target->isWidget()) { + if (result.isOverWidget() && target && target->isWidget()) { Widget* widget = static_cast(target)->widget(); if (widget && passWheelEventToWidget(e, widget)) { diff --git a/WebCore/page/MouseEventWithHitTestResults.h b/WebCore/page/MouseEventWithHitTestResults.h index 574e644905c..c4e419c7251 100644 --- a/WebCore/page/MouseEventWithHitTestResults.h +++ b/WebCore/page/MouseEventWithHitTestResults.h @@ -39,6 +39,7 @@ public: const IntPoint localPoint() const; Scrollbar* scrollbar() const; bool isOverLink() const; + bool isOverWidget() const { return m_hitTestResult.isOverWidget(); } private: PlatformMouseEvent m_event; diff --git a/WebCore/rendering/HitTestResult.cpp b/WebCore/rendering/HitTestResult.cpp index 1980c6c6222..53b376d8521 100644 --- a/WebCore/rendering/HitTestResult.cpp +++ b/WebCore/rendering/HitTestResult.cpp @@ -42,6 +42,7 @@ using namespace HTMLNames; HitTestResult::HitTestResult(const IntPoint& point) : m_point(point) + , m_isOverWidget(false) { } @@ -52,6 +53,7 @@ HitTestResult::HitTestResult(const HitTestResult& other) , m_localPoint(other.localPoint()) , m_innerURLElement(other.URLElement()) , m_scrollbar(other.scrollbar()) + , m_isOverWidget(other.isOverWidget()) { } @@ -67,6 +69,7 @@ HitTestResult& HitTestResult::operator=(const HitTestResult& other) m_localPoint = other.localPoint(); m_innerURLElement = other.URLElement(); m_scrollbar = other.scrollbar(); + m_isOverWidget = other.isOverWidget(); return *this; } diff --git a/WebCore/rendering/HitTestResult.h b/WebCore/rendering/HitTestResult.h index bf76a609891..5ed9b34fd21 100644 --- a/WebCore/rendering/HitTestResult.h +++ b/WebCore/rendering/HitTestResult.h @@ -49,6 +49,7 @@ public: IntPoint localPoint() const { return m_localPoint; } Element* URLElement() const { return m_innerURLElement.get(); } Scrollbar* scrollbar() const { return m_scrollbar.get(); } + bool isOverWidget() const { return m_isOverWidget; } void setToNonShadowAncestor(); @@ -58,6 +59,7 @@ public: void setLocalPoint(const IntPoint& p) { m_localPoint = p; } void setURLElement(Element*); void setScrollbar(Scrollbar*); + void setIsOverWidget(bool b) { m_isOverWidget = b; } Frame* targetFrame() const; IntRect boundingBox() const; @@ -82,6 +84,7 @@ private: // determine where inside the renderer we hit on subsequent operations. RefPtr m_innerURLElement; RefPtr m_scrollbar; + bool m_isOverWidget; // Returns true if we are over a widget (and not in the border/padding area of a RenderWidget for example). }; String displayString(const String&, const Node*); diff --git a/WebCore/rendering/RenderWidget.cpp b/WebCore/rendering/RenderWidget.cpp index a377ad11a80..9fb4fe610b0 100644 --- a/WebCore/rendering/RenderWidget.cpp +++ b/WebCore/rendering/RenderWidget.cpp @@ -33,6 +33,7 @@ #include "EventNames.h" #include "FrameView.h" #include "GraphicsContext.h" +#include "HitTestResult.h" #include "RenderLayer.h" #include "RenderView.h" @@ -269,4 +270,15 @@ RenderWidget* RenderWidget::find(const Widget* widget) return widgetRendererMap().get(widget); } +bool RenderWidget::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, int x, int y, int tx, int ty, HitTestAction action) +{ + bool hadResult = result.innerNode(); + bool inside = RenderReplaced::nodeAtPoint(request, result, x, y, tx, ty, action); + + // Check to see if we are really over the widget itself (and not just in the border/padding area). + if (inside && !hadResult && result.innerNode() == element()) + result.setIsOverWidget(contentBox().contains(result.localPoint())); + return inside; +} + } // namespace WebCore diff --git a/WebCore/rendering/RenderWidget.h b/WebCore/rendering/RenderWidget.h index 436d62fec27..0552cb5aba5 100644 --- a/WebCore/rendering/RenderWidget.h +++ b/WebCore/rendering/RenderWidget.h @@ -56,6 +56,8 @@ public: virtual void setWidget(Widget*); + virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, int x, int y, int tx, int ty, HitTestAction); + private: void resizeWidget(Widget*, int w, int h); -- GitLab