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

Source/core/rendering/RenderBlock.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 2864 matching lines @@
         && isPointInOverflowControl(result, locationInContainer.point(), adjustedLocation)) {
         updateHitTestResult(result, locationInContainer.point() - localOffset);
         // FIXME: isPointInOverflowControl() doesn't handle rect-based tests yet.
         if (!result.addNodeToRectBasedTestResult(nodeForHitTest(), request, locationInContainer))
            return true;
     }
 
     // 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);
         if (hasOverflowClip())
             scrolledOffset -= scrolledContentOffset();
 
         // Hit test contents if we don't have columns.
         if (!hasColumns()) {
             if (hitTestContents(request, result, locationInContainer, toLayoutPoint(scrolledOffset), hitTestAction)) {
                 updateHitTestResult(result, flipForWritingMode(locationInContainer.point() - localOffset));
@@ skipping 2125 matching lines @@
 void RenderBlock::showLineTreeAndMark(const InlineBox* markedBox1, const char* markedLabel1, const InlineBox* markedBox2, const char* markedLabel2, const RenderObject* obj) const
 {
     showRenderObject();
     for (const RootInlineBox* root = firstRootBox(); root; root = root->nextRootBox())
         root->showLineTreeAndMark(markedBox1, markedLabel1, markedBox2, markedLabel2, obj, 1);
 }
 
 #endif
 
 } // namespace WebCore