Bug 99656

Created attachment 169298 [details] Patch

This patch comes from 6034d2de096bbdaf1be3474b0141c4d57094a956 in the chromium-android branch (for those with access to that source tree).

if (!renderer()->hitTest(request, result, hitTestLocation, toLayoutPoint(layerBounds.location() - renderBoxLocation()), hitTestFilter)) { // It's wrong to set innerNode, but then claim that you didn't hit anything, unless it is // a rect-based test. ASSERT(!result.innerNode() || (result.isRectBasedTest() && result.rectBasedTestResult().size())); return false; } Apparently that is the ASSERT that fires.

Created attachment 169313 [details] Some attempts at a test

Comment on attachment 169298 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169298&action=review The patch makes sense. This is is one of those cases that makes me think addNodeToRectBasedResult really should be called from inside updateHitTestResult(). > Source/WebCore/rendering/RenderBlock.h:570 > + Node* nodeForHitTest(); Could be const.

Created attachment 171151 [details] test case I could repro this issue on android JB stock browser and chromium content shell(I had some weeks old content shell code but not the latest one) with attached small test case. You need to touch exactly at the border of image element. Looks to be a problem when hit test point falls in anonymous block. As Adam rightly pointed out, Assert is because we are setting inner node of HitTestResult and also saying that HitTest doesn't found anything by returning false in HitTest function which is incorrect. This is happening because we are using different nodes in RenderBlock::nodeAtPoint and when setting innerNode in RenderBlock::updateHitTestResult. In RenderBlock::updateHitTestResult, in case of anonymous block continuation, we are using continuation()->node() which returns enclosing node i.e. <a> in our case. But in RenderBlock::nodeAtPoint, while calling addNodeToRectBasedTestResult we use "node()" which returns NULL for anonymous blocks. Using same node at both the places fixes the problem. Patch on this bug looks good to me, Antonio can confirm this as from file revisions, looks like mainly he had implememented rect based hittest.

cc Antonio

Comment on attachment 169298 [details] Patch could not Internals::nodesFromRect be used to trigger this assert?

Comment on attachment 169298 [details] Patch I'm going to try to construct a test based on the comments above.

Created attachment 172085 [details] LayoutTest that hits the ASSERT

Comment on attachment 169298 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169298&action=review >> Source/WebCore/rendering/RenderBlock.h:570 >> + Node* nodeForHitTest(); > > Could be const. Done.

CC'ing some BlackBerry people, since it gets hitten for them as well.

Created attachment 172087 [details] Patch

Comment on attachment 172087 [details] Patch Yay!

Comment on attachment 172087 [details] Patch Clearing flags on attachment: 172087 Committed r133330: <http://trac.webkit.org/changeset/133330>

All reviewed patches have been landed. Closing bug.

Comment on attachment 172087 [details] Patch Nice fix!

Thanks. :)