[Webkit-unassigned] [Bug 40197] Enhance the hit testing to take a rectangle instead of a point

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=40197





--- Comment #41 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-07-19 20:06:30 PST ---
(From update of attachment 62029)
LayoutTests/fast/dom/resources/nodesFromRect.js:15
 +    var nodes = document.nodesFromRect(x, y, hPadding, vPadding, true /* ignoreClipping */);
isn't this centerX and centerY? 

WebCore/dom/Document.cpp:1038
 +  // * make it receive a real Rect, i.e. nodesFromRect(x, y, w, h);
why Rect with uppercase?

WebCore/dom/Document.cpp:1039
 +  // * make it receive the expading size of each direction separatedly,
a d too much in separately

WebCore/dom/Document.cpp:1055
 +      float zoomFactor = frameView->pageZoomFactor();
What about scale? The QGraphicsWebView can have a scale set which will be applied to the QPainter - will that work?

WebCore/dom/Document.cpp:1056
 +      IntPoint point = roundedIntPoint(FloatPoint(x * zoomFactor  + view()->scrollX(), y * zoomFactor + view()->scrollY()));
Is there any need to specify that it is an int point?

WebCore/dom/Document.cpp:1059
 +      if (!ignoreClipping && !frameView->visibleContentRect().intersects(IntRect(point, padding)))
Maybe a simple comment over complex if's like this would make the code a bit easily understandable

WebCore/dom/Document.h:305
 +      PassRefPtr<NodeList> nodesFromRect(int x, int y, int hPadding, int vPadding, bool ignoreClipping) const;
centerX, centerY?

WebCore/page/EventHandler.h:108
 +      HitTestResult hitTestResultAtPoint(const IntPoint&, bool allowShadowContent, bool ignoreClipping = false, HitTestScrollbars scrollbars = DontHitTestScrollbars, const IntSize& padding = IntSize(-1, -1));
I believe that we have something like IntPoint::zero() or zeroPoint or so, maybe we want a special constructor for this? Darin?

WebCore/rendering/HitTestResult.cpp:56
 +  HitTestResult::HitTestResult(const IntPoint& point, const IntSize& padding)
centerPoint?

WebCore/rendering/HitTestResult.h:45
 +      // hit test.
Just write that on one line

WebCore/rendering/HitTestResult.h:59
 +      bool isRegionTest() const { return m_isRegionTest && hasValidPadding(); }
You talk about rect based hit testing, and here you call it region? Is it region or is it rect?

WebCore/rendering/HitTestResult.h:97
 +      inline bool hasValidPadding() const { return m_padding.width() >= 0 && m_padding.height() >= 0; }
wouldn't hasPadding be sufficient? Padding, region, rect... it is a bit confusing

WebCore/rendering/RenderBlock.cpp:3756
 +          // TODO: isPointInOverflowControl() doesn't handle region test yet.
I think we only allow FIXME:

WebCore/rendering/RenderBlock.cpp:3847
 +                  hitTestContents(request, result, x, y, finalX, finalY, hitTestAction);
no return needed here? 

WebCore/rendering/RenderLineBoxList.cpp:256
 +          if (y + result.padding().height() >= ty + curr->root()->topVisibleOverflow() && y - result.padding().height() < ty + curr->root()->bottomVisibleOverflow()) {
there lines are quite long

WebCore/rendering/RenderSVGRoot.cpp:323
 +              // TODO: nodeAtFloatPoint() doesn't handle region test yet.
I believe we only use FIXME

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.