Bug 110741 – [WK2][EFL] WebView: Add callbacks to the WKViewClient to remove direct access to page viewport controller

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Mikhail Pozdnyakov]

SSIA. WKViewClient interface shall be extended.

[Mikhail Pozdnyakov]

Created attachment 190036 [details]
patch

[Kenneth Rohde Christiansen]

Comment on attachment 190036 [details]
patch

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

> Source/WebKit2/UIProcess/API/C/efl/WKView.h:56
> +    WKViewCallback                                   pageTransitionViewportReady;

Maybe we could find a better name that users could actually understand :-) I have no suggestion though.

> Source/WebKit2/UIProcess/efl/WebViewClient.h:48
> +    void pageTransitionViewportReady(WebView*);

When exactly does this happen? Isnt it when the new page is ready to render something?

Something with did?

[Build Bot]

Comment on attachment 190036 [details]
patch

Attachment 190036 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16745259

New failing tests:
svg/zoom/page/zoom-mask-with-percentages.svg

[Mikhail Pozdnyakov]

(In reply to comment #2)
> (From update of attachment 190036 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=190036&action=review
> 
> > Source/WebKit2/UIProcess/API/C/efl/WKView.h:56
> > +    WKViewCallback                                   pageTransitionViewportReady;
> 
> Maybe we could find a better name that users could actually understand :-) I have no suggestion though.
> 
> > Source/WebKit2/UIProcess/efl/WebViewClient.h:48
> > +    void pageTransitionViewportReady(WebView*);
> 
> When exactly does this happen? Isnt it when the new page is ready to render something?
yeah
> 
> Something with did?
I guess we could take after WebPage and use "didCompletePageTransition".

[Mikhail Pozdnyakov]

Created attachment 190219 [details]
patch v2

WebViewClient pageTransitionViewportReady -> didCompletePageTransition.

[Kenneth Rohde Christiansen]

LGTM

[Kenneth Rohde Christiansen]

Comment on attachment 190219 [details]
patch v2

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

> Source/WebKit2/UIProcess/API/C/efl/WKView.h:44
> +typedef void (*WKViewPageDidRequestScrollCallback)(WKViewRef view, WKPoint position, const void* clientInfo);

Wouldn't something like didChangePosition not make more sense?

[Caio Marcelo de Oliveira Filho]

Comment on attachment 190219 [details]
patch v2

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

>> Source/WebKit2/UIProcess/API/C/efl/WKView.h:44
>> +typedef void (*WKViewPageDidRequestScrollCallback)(WKViewRef view, WKPoint position, const void* clientInfo);
> 
> Wouldn't something like didChangePosition not make more sense?

I think the DidRequestScroll is fine. The scroll was requested, the view user may decide to do different things besides setting the position like not scrolling at all, or scroll the page in an animated fashion. While WKView shouldn't automatically handle those things, it's fine for the EWK specific view to handle them.

[Kenneth Rohde Christiansen]

(In reply to comment #8)
> (From update of attachment 190219 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=190219&action=review
> 
> >> Source/WebKit2/UIProcess/API/C/efl/WKView.h:44
> >> +typedef void (*WKViewPageDidRequestScrollCallback)(WKViewRef view, WKPoint position, const void* clientInfo);
> > 
> > Wouldn't something like didChangePosition not make more sense?
> 
> I think the DidRequestScroll is fine. The scroll was requested, the view user may decide to do different things besides setting the position like not scrolling at all, or scroll the page in an animated fashion. While WKView shouldn't automatically handle those things, it's fine for the EWK specific view to handle them.

The problem is that in some cases we automatically adjust the position on the web process site, so it is not really a request.

[Caio Marcelo de Oliveira Filho]

> The problem is that in some cases we automatically adjust the position on the web process site, so it is not really a request.

Which cases?

[Mikhail Pozdnyakov]

(In reply to comment #10)
> > The problem is that in some cases we automatically adjust the position on the web process site, so it is not really a request.
> 
> Which cases?

For instance when viewport attributes are changing, please take a look at WebPage::sendViewportAttributesChanged.

[Mikhail Pozdnyakov]

Created attachment 196856 [details]
patch v3

Rebased. didRequestScroll -> didChangeContentsPosition

[Mikhail Pozdnyakov]

Committed r147913: <http://trac.webkit.org/changeset/147913>