Bug 125752 - WebKit2 View Gestures: Implement smartMagnifyWithEvent: and make it work
Summary: WebKit2 View Gestures: Implement smartMagnifyWithEvent: and make it work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-14 20:14 PST by Tim Horton
Modified: 2013-12-20 13:58 PST (History)
4 users (show)

See Also:


Attachments
patch (17.25 KB, patch)
2013-12-20 04:25 PST, Tim Horton
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2013-12-14 20:14:10 PST
This is 'double-tap-to-zoom-on-an-element'.
Comment 1 Tim Horton 2013-12-14 20:39:21 PST
<rdar://problem/15664245>
Comment 2 Tim Horton 2013-12-20 04:25:28 PST
Created attachment 219750 [details]
patch
Comment 3 Anders Carlsson 2013-12-20 12:45:23 PST
Comment on attachment 219750 [details]
patch

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

> Source/WebKit2/WebProcess/WebPage/ViewGestureGeometryCollector.h:30
> +#include <WebCore/FloatPoint.h>

No need to include this here, just forward declare FloatPoint.
Comment 4 Simon Fraser (smfr) 2013-12-20 12:51:48 PST
Comment on attachment 219750 [details]
patch

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

> Source/WebKit2/UIProcess/mac/ViewGestureController.cpp:142
> +static float maximumRectangleComponentDelta(FloatRect a, FloatRect b)

Does not compute.

> Source/WebKit2/UIProcess/mac/ViewGestureController.cpp:147
> +void ViewGestureController::didCollectGeometryForSmartMagnificationGesture(FloatPoint origin, FloatRect renderRect, FloatRect visibleContentRect, bool elementIsReplaced)

Maybe isReplacedElement to make it a bit clearer that this is using the CSS "replaced" terminology.

> Source/WebKit2/UIProcess/mac/ViewGestureController.cpp:149
> +    double currentMagnification = m_webPageProxy.pageScaleFactor();

currentScaleFactor?

> Source/WebKit2/WebProcess/WebPage/ViewGestureGeometryCollector.cpp:74
> +    HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowShadowContent);
> +    HitTestResult hitTestResult = HitTestResult(scrolledOrigin);
> +    m_webPage.mainFrameView()->renderView()->hitTest(request, hitTestResult);
> +
> +    if (hitTestResult.innerNode()) {
> +        bool isReplaced;
> +        FloatRect renderRect = hitTestResult.innerNode()->renderRect(&isReplaced);
> +        m_webPage.send(Messages::ViewGestureController::DidCollectGeometryForSmartMagnificationGesture(origin, renderRect, visibleContentRect, isReplaced));
> +    }

Does this find things in subframes?
Comment 5 Tim Horton 2013-12-20 13:08:29 PST
(In reply to comment #4)
> (From update of attachment 219750 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=219750&action=review
> 
> > Source/WebKit2/UIProcess/mac/ViewGestureController.cpp:142
> > +static float maximumRectangleComponentDelta(FloatRect a, FloatRect b)
> 
> Does not compute.

We talked about this on IRC. I agree!

> > Source/WebKit2/UIProcess/mac/ViewGestureController.cpp:147
> > +void ViewGestureController::didCollectGeometryForSmartMagnificationGesture(FloatPoint origin, FloatRect renderRect, FloatRect visibleContentRect, bool elementIsReplaced)
> 
> Maybe isReplacedElement to make it a bit clearer that this is using the CSS "replaced" terminology.

Sure.

> > Source/WebKit2/UIProcess/mac/ViewGestureController.cpp:149
> > +    double currentMagnification = m_webPageProxy.pageScaleFactor();
> 
> currentScaleFactor?

Sure.

> > Source/WebKit2/WebProcess/WebPage/ViewGestureGeometryCollector.cpp:74
> > +    HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowShadowContent);
> > +    HitTestResult hitTestResult = HitTestResult(scrolledOrigin);
> > +    m_webPage.mainFrameView()->renderView()->hitTest(request, hitTestResult);
> > +
> > +    if (hitTestResult.innerNode()) {
> > +        bool isReplaced;
> > +        FloatRect renderRect = hitTestResult.innerNode()->renderRect(&isReplaced);
> > +        m_webPage.send(Messages::ViewGestureController::DidCollectGeometryForSmartMagnificationGesture(origin, renderRect, visibleContentRect, isReplaced));
> > +    }
> 
> Does this find things in subframes?

No, nor does the existing implementation. We just zoom on the frame instead. Possible room for improvement! (I actually had it returning subframe results at some point, but not mapping the points into the root document space, so it would zoom somewhere wacky).
Comment 6 Tim Horton 2013-12-20 13:58:23 PST
http://trac.webkit.org/changeset/160923