hitTest should take care if the location is inside rounded border if border-radius is specified

Source/core/rendering/RenderBlock.cpp

Index: Source/core/rendering/RenderBlock.cpp
diff --git a/Source/core/rendering/RenderBlock.cpp b/Source/core/rendering/RenderBlock.cpp
index fbcc31eb3c1ddbed2c55e49fdf3f75103d6adc04..00db592272f0d6224120b775499de735633d9833 100644
--- a/Source/core/rendering/RenderBlock.cpp
+++ b/Source/core/rendering/RenderBlock.cpp
@@ -2882,7 +2882,8 @@ bool RenderBlock::nodeAtPoint(const HitTestRequest& request, HitTestResult& resu
     // If we have clipping, then we can't have any spillout.
     bool useOverflowClip = hasOverflowClip() && !hasSelfPaintingLayer();
     bool useClip = (hasControlClip() || useOverflowClip);
-    bool checkChildren = !useClip || (hasControlClip() ? locationInContainer.intersects(controlClipRect(adjustedLocation)) : locationInContainer.intersects(overflowClipRect(adjustedLocation, IncludeOverlayScrollbarSize)));
+    LayoutRect clipRect = overflowClipRect(adjustedLocation, IncludeOverlayScrollbarSize);
+    bool checkChildren = !useClip || (hasControlClip() ? locationInContainer.intersects(controlClipRect(adjustedLocation)) : (!isRenderView() && style()->hasBorderRadius() ? locationInContainer.intersects(style()->getRoundedBorderFor(clipRect)) : locationInContainer.intersects(clipRect)));

[ojan, 2014/04/30 18:37:47]

Why the !isRenderView check? I see that the code below also has that check, but
I don't understand why it's necessary (here or below).

Also, this line is getting a bit unwieldy. We generally avoid nested ternaries.
You could you instead write it as a series of if-statements, e.g. something like

bool checkChildren = !useClip;
if (!checkChildren) {
    if (hasControlClip()) {
        checkChildren =
locationInContainer.intersects(controlClipRect(adjustedLocation));
    } else {
        if (!isRenderView() && style()->hasBorderRadius())
            ...
        else
            ...
    }
}

[pals, 2014/05/06 06:33:41]

Done. I have removed the !isRenderView() checks. There is no LayoutTests
failures but I am not sure of this change.

[ojan, 2014/05/07 03:11:03]

Can you look at the history for why this check was added in the first place?
It'd be good to understand that to have confidence that we're not regressing
something. I think removing the checks makes sense, but I just want to be more
sure.

[pals, 2014/05/07 06:31:53]

The check was first added at
http://trac.webkit.org/changeset/8070/trunk/WebCore/khtml/rendering/render_bl...
and modified here
http://trac.webkit.org/changeset/40258/trunk/WebCore/rendering/RenderBlock.cpp
And then this check was carried forward here
http://trac.webkit.org/changeset/147635/trunk/Source/WebCore/rendering/Render...

I think this check was added keeping in mind that root element/RenderView cannot
have border-radius. But i am not sure how we could specify border-radius CSS
property for root element in the first place. So, I think its good to remove
this check.

     if (checkChildren) {
         // Hit test descendants first.
         LayoutSize scrolledOffset(localOffset);