Bug 125752 – WebKit2 View Gestures: Implement smartMagnifyWithEvent: and make it work

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Tim Horton]

This is 'double-tap-to-zoom-on-an-element'.

[Tim Horton]

<rdar://problem/15664245>

[Tim Horton]

Created attachment 219750 [details]
patch

[Anders Carlsson]

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.

[Simon Fraser (smfr)]

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?

[Tim Horton]

(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).

[Tim Horton]

http://trac.webkit.org/changeset/160923