Bug 82609 - [Qt] Find zoomable area using area-based hit-testing
Summary: [Qt] Find zoomable area using area-based hit-testing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-29 06:50 PDT by Allan Sandfeld Jensen
Modified: 2012-03-30 08:47 PDT (History)
11 users (show)

See Also:


Attachments
Patch (16.80 KB, patch)
2012-03-29 06:57 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (23.40 KB, patch)
2012-03-30 05:21 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Sandfeld Jensen 2012-03-29 06:50:14 PDT
The current implementation of tap-to-zoom uses a normal hit-testing and simply walks the DOM-tree to find a suitable zoomable area.

This fails to catch almost all cases of CSS layout that doesn't follow flow. A better solution is to use the areas returned by a rect-based hit-testing and returning the best zoomable area from the result.
Comment 1 Allan Sandfeld Jensen 2012-03-29 06:57:28 PDT
Created attachment 134570 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-03-29 07:28:55 PDT
Comment on attachment 134570 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=134570&action=review

lets look this over together tomorrow

> Source/WebCore/page/TouchAdjustment.cpp:101
> +    ASSERT(node->renderer());
> +    return node->renderer()->isBox();
> +}

What happens if it has a rotation transform? bounding box ?

> Source/WebCore/page/TouchAdjustment.cpp:120
> +    RenderBox* renderer = static_cast<RenderBox*>(node->renderer());

I believe there is a toRenderBox method
Comment 3 Allan Sandfeld Jensen 2012-03-30 02:33:10 PDT
(In reply to comment #2)
> (From update of attachment 134570 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=134570&action=review
> 
> lets look this over together tomorrow
> 
> > Source/WebCore/page/TouchAdjustment.cpp:101
> > +    ASSERT(node->renderer());
> > +    return node->renderer()->isBox();
> > +}
> 
> What happens if it has a rotation transform? bounding box ?
> 
Yes. This is similar to the current code. Also in this case bounding box is not an approximation. The bounding box is truly what we want to zoom to. I don't think UI's are ready for zoom and rotate :D

> > Source/WebCore/page/TouchAdjustment.cpp:120
> > +    RenderBox* renderer = static_cast<RenderBox*>(node->renderer());
> 
> I believe there is a toRenderBox method
Right!
Comment 4 Allan Sandfeld Jensen 2012-03-30 05:21:30 PDT
Created attachment 134789 [details]
Patch
Comment 5 Kenneth Rohde Christiansen 2012-03-30 05:34:12 PDT
Comment on attachment 134789 [details]
Patch

Nice improvement with layout tests!
Comment 6 WebKit Review Bot 2012-03-30 08:47:19 PDT
Comment on attachment 134789 [details]
Patch

Clearing flags on attachment: 134789

Committed r112669: <http://trac.webkit.org/changeset/112669>
Comment 7 WebKit Review Bot 2012-03-30 08:47:24 PDT
All reviewed patches have been landed.  Closing bug.