Bug 94936 – Improve touch-adjustment at an iframe boundary.

• Component: WebCore Misc.
• Status: RESOLVED DUPLICATE

[Kevin Ellis]

Rect-based hit-tests largely follow the same logic as point based tests returning once a hit layer is found unless conditions are met for depth sorting descendants.  While this is entirely appropriate for point-based tests, it does not allow a rect-based test to consider all possible elements when layers at different z-orders partially overlap the hit-test area.

This limitation can be problematic for touch adjustment if a small control such as a checkbox is close to the edge of a navigation bar at a higher z-index.  A touch that covers the checkbox can easily clip the edge of the navigation bar, which would cause the hit test to only return elements on the navigation bar in the hit-test results even if the center of the touch lies outside the boundary of the navigation bar.

[Allan Sandfeld Jensen]

I thought RenderLayer::hitTestLayer already handled that. A layer is only considered hit (and thus terminating the search) if an element fully encloses the hit-tested touch-point. 

This behaviour does create another bug of returning too many elements, since the areas that have been hit are not subtracted from the hit-test and can continue to hit elements in layers below.

[Antonio Gomes]

See FatFingers.cpp 

354             
355 bool FatFingers::checkForClickableElement(Element* curElement,
356                                           Vector<IntersectingRegion>& intersectingRegions,
357                                           Platform::IntRectRegion& remainingFingerRegion,
358                                           RenderLayer*& lowestPositionedEnclosingLayerSoFar)

our intersection area is a region. Every time we have a higher zindex'ed layer, we subtract that layer's area from the "remaining intersection region", once we are done with it.

[Kevin Ellis]

(In reply to comment #1)
> I thought RenderLayer::hitTestLayer already handled that. A layer is only considered hit (and thus terminating the search) if an element fully encloses the hit-tested touch-point. 
> 
> This behaviour does create another bug of returning too many elements, since the areas that have been hit are not subtracted from the hit-test and can continue to hit elements in layers below.

Will write up a test case demonstrating the problem.  Here is a brief description of the page layout.  Have a series of absolute positioned elements each covering the full page, but with the active page having a higher z-index.  Have a left side bar that overlaps the page with a higher z-index.  The active page contains checkboxes that are closely aligned to the navigation bar.

I misinterpreted some of the diagnostic output in my original test.  When the touch overlaps a checkbox but clips the navigation bar, we see the navigation bar and active page in the hit test results, but not the checkbox.  So the results are not restricted to the highest z-index element as I originally thought; however, the search is not picking up all of the elements that overlap the hit-test area.  The hit-test scores are pretty meaningless given the size of the elements involved and it is simply pulling to the smaller one (the navigation bar) based on the relative scores.

[Antonio Gomes]

Created attachment 160473 [details]
testcasE?

[Kevin Ellis]

Created attachment 160722 [details]
Example where it is difficult to tap on a checkbox.  

Attached an example that demonstrates a case where it is tough to tap on a checkbox even if touch adjustment is enabled.  The z-order issue initially reported might be a red-herring.  It turned out to also be important for the navigation bar and page to be in separate <iframe>s.

How does rect-based hit testing work when it spans multiple <iframe>s?

[Antonio Gomes]

> How does rect-based hit testing work when it spans multiple <iframe>s?

it adds the <iframe> to the list of reached nodes, iirc.

[Kevin Ellis]

That is consistent with what I'm seeing for the example.  The <iframe>s are detected but do not provide enough information for touch adjustment since smaller elements inside the <iframe>s are not picked up as candidates.

One solution may be to repeat the hit test on each of the <iframe>s from the first test.  Alternatively, could repeat the hit-test just for the <iframe> that contains the midpoint of the touch.

[Antonio Gomes]

(In reply to comment #7)
> That is consistent with what I'm seeing for the example.  The <iframe>s are detected but do not provide enough information for touch adjustment since smaller elements inside the <iframe>s are not picked up as candidates.
> 
> One solution may be to repeat the hit test on each of the <iframe>s from the first test.  Alternatively, could repeat the hit-test just for the <iframe> that contains the midpoint of the touch.

Right. this is what we do for blackberry port.

FatFingers.cpp:
...
335         bool isElement = curNode->isElementNode();
336         if (isElement && isValidFrameOwner(toElement(curNode))) {
337 
338             HTMLFrameOwnerElement* owner = static_cast<HTMLFrameOwnerElement*>(curNode);
339             Document* childDocument = owner && owner->contentFrame() ? owner->contentFrame()->document() : 0;
340             if (!childDocument)
341                 continue;
342 
343             ASSERT(childDocument->frame()->view());
344 
345             foundOne |= findIntersectingRegions(childDocument, intersectingRegions, remainingFingerRegion);

[Allan Sandfeld Jensen]

(In reply to comment #7)
> That is consistent with what I'm seeing for the example.  The <iframe>s are detected but do not provide enough information for touch adjustment since smaller elements inside the <iframe>s are not picked up as candidates.
> 
> One solution may be to repeat the hit test on each of the <iframe>s from the first test.  Alternatively, could repeat the hit-test just for the <iframe> that contains the midpoint of the touch.

I think, though I am not entirely sure, that touch-adjustment is automatically performed in the frame that contains the midpoint, because it will be the frame receiving the event. Testing down into iframes contained in the result would help with the case where the hot-spot is in the top frame, but would probably still give problems when the hot-spot is in a lower frame.

[Allan Sandfeld Jensen]

The problem is in EventHandler::hitTestResultAtPoint, it descends into inner frames if the frame is the HitTestResult::innerNode. 

Unfortunately the innerNode is rather useless with rect-based tests, and is often just the first intersected node.

The underlying problem is that EventHandler::hitTestResultAtPoint is not that suitable for rect-based tests. 

I think we should remove the padding argument from EventHandler::hitTestResultAtPoint, so it can't be used for area based tests, and provide a EventHandler::hitTestResultAtRect function to replace it.

[Allan Sandfeld Jensen]

Created attachment 160953 [details]
Quick Patch

Here is a quick experimental patch that solves the test-case.

I am not sure it the right way to do it though. This won't handle iframes that have been transformed for instance. To handle that we need to let the hit-test descend into inner-documents during the normal hit-test recursion, and add a parameter similar to AllowShadowContent, AllowInnerDocumentContent, that controls whether it should go into inner documents.

[Kevin Ellis]

Created attachment 160991 [details]
Patch

[Kevin Ellis]

Still some failures in the test case to resolve.  The approach taken was to refine the scoring to favor the candidate containing the target hotspot in the event that all candidate scores are poor.  A poor score is denoted by a distance to center score being greater than 1 implying that the touch did not overlap the center-line even though the target is larger than the touch area (in which case a much stronger hit is expected).  

The strategy differs somewhat from the approach used in zoomableIntersectionQuotient which assigns an infinite score if the touch hotspot is not contained.  The rationale is that it is easy to get very good coverage but have the hotspot lie outside the candidate for a small target.  For large targets, there is still some merit in touch adjustment if none of the candidates contain the hotspot.

[Allan Sandfeld Jensen]

(In reply to comment #12)
> Created an attachment (id=160991) [details]
> Patch

Does this require the patch I attached (or the one in bug #95204), and if not, how would it overcome the issue of only getting the elements of one of the frames?

[Kevin Ellis]

(In reply to comment #14)
> (In reply to comment #12)
> > Created an attachment (id=160991) [details] [details]
> > Patch
> 
> Does this require the patch I attached (or the one in bug #95204), and if not, how would it overcome the issue of only getting the elements of one of the frames?

Yes, all test cases pass with the patch for #95204.  On its own, this patch only partially addresses the issue of getting elements from only one frame.  It ensures (with some reasonable level of confidence) that the correct iframe is selected when the hit adjustment candidates are iframes, but does not resume touch adjustment on the position in the child iframe.

[Kevin Ellis]

Created attachment 161048 [details]
Patch

[Kevin Ellis]

Backed out TouchAdjustment tweak and updated expected test results.  Test contains expected failures which are fixed by patch 95204.

[Allan Sandfeld Jensen]

(In reply to comment #17)
> Backed out TouchAdjustment tweak and updated expected test results.  Test contains expected failures which are fixed by patch 95204.

Oh nice. I will merge the bugs then and include your test in my patch (with attribution of course!).

[Allan Sandfeld Jensen]

*** This bug has been marked as a duplicate of bug 95204 ***