Bug 110463 – [WK2][EFL] WebView: Add callbacks to the WKViewClient to handle page viewport update

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Mikhail Pozdnyakov]

SSIA.

[Mikhail Pozdnyakov]

Created attachment 189748 [details]
WIP

[Kenneth Rohde Christiansen]

Comment on attachment 189748 [details]
WIP

Looks nice... im not sure whether we should use these names or the naming from css device adapt spec... scale seems nicer though than zoom as the spec uses

[Mikhail Pozdnyakov]

(In reply to comment #2)
> (From update of attachment 189748 [details])
> Looks nice... im not sure whether we should use these names or the naming from css device adapt spec... scale seems nicer though than zoom as the spec uses

good point, I would align names in public apis with specs, maybe we should rename WebCore types accordingly.

[Mikhail Pozdnyakov]

Created attachment 189765 [details]
patch

Decided not to include WKViewportAttributes functions so far, as they are not used within this patch (will be used
when Page Viewport Controller is based on C API). In the interim we can figure out appropriate naming for these functions.

[Kenneth Rohde Christiansen]

Comment on attachment 189765 [details]
patch

LGTM

[Benjamin Poulain]

Comment on attachment 189765 [details]
patch

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

> Source/WebKit2/UIProcess/API/C/efl/WKView.h:52
>      WKViewPageDidChangeContentsSizeCallback          didChangeContentsSize;
> +    WKViewPageDidChangeViewportAttributesCallback    didChangeViewportAttributes;

I am more and more confused by what is going on in EFL :(

What goes in view? What goes in page? Why are those two suddenly view callbacks?

[Mikhail Pozdnyakov]

(In reply to comment #6)
> (From update of attachment 189765 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=189765&action=review
> 
> > Source/WebKit2/UIProcess/API/C/efl/WKView.h:52
> >      WKViewPageDidChangeContentsSizeCallback          didChangeContentsSize;
> > +    WKViewPageDidChangeViewportAttributesCallback    didChangeViewportAttributes;
> 
> I am more and more confused by what is going on in EFL :(
> 
> What goes in view? What goes in page? Why are those two suddenly view callbacks?

We want to keep WebView toolkit-agnostic and WKViewClient in general is needed
to provide a way for enabling EwkView handlers.

ViewportAttributes are calculated based on view size and view in many ways controls the viewport, so I think having didChangeViewportAttributes callback in WKViewClient is quite natural.

[Benjamin Poulain]

Wait what? What do you think a WebPage is then? And the PageUIClient?

[Mikhail Pozdnyakov]

(In reply to comment #8)
> Wait what? What do you think a WebPage is then? And the PageUIClient?

Please correct me if I'm wrong, in my understanding WebPage is what to be shown by WebView and PageUIClient is to provide UI-related activities required by  WebPage (like showing message box or setting a focus).

[Mikhail Pozdnyakov]

Created attachment 197269 [details]
rebased

[Jesus Sanchez-Palencia]

(In reply to comment #6)
> I am more and more confused by what is going on in EFL :(
> 
> What goes in view? What goes in page? Why are those two suddenly view callbacks?

Benjamin, this idea came from the way Nix decided to handle this sort of information. It is not only about being "toolkit-agnostic", but also about being "UX-agnostic". NIXView also gets the same callbacks (https://github.com/WebKitNix/webkitnix/blob/master/Source/WebKit2/UIProcess/API/nix/NIXView.h#L82) so the application can decide how it wants to handle viewport changes.

In our Minibrowser, for instance, we use this so we can handle "gesture"-scaling properly, etc. https://github.com/WebKitNix/webkitnix/blob/master/Tools/MiniBrowser/nix/main.cpp#L689

I'm more than interested to hear your feedback on this.

(In reply to comment #9)
> (In reply to comment #8)
> > Wait what? What do you think a WebPage is then? And the PageUIClient?
> 
> Please correct me if I'm wrong, in my understanding WebPage is what to be shown by WebView and PageUIClient is to provide UI-related activities required by  WebPage (like showing message box or setting a focus).

I'm also interested to check if this is accurate or not. :)

[WebKit Commit Bot]

Comment on attachment 197269 [details]
rebased

Clearing flags on attachment: 197269

Committed r148274: <http://trac.webkit.org/changeset/148274>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Benjamin Poulain]

This was a bad idea.

I am in favor of reverting this change. RAW viewport attribute should not be exposed. What should be exposed is the behavior computed with those attributes.

[Jesus Sanchez-Palencia]

So far, Nix needs such viewport information on its API layer so the application can use that information to implement whatever is the way it wants to interact with the page. We don't use the PageViewportController as the other CoordinatedGraphics ports do.

However, currently WKViewClient is exposing a WKViewportAttribute which is useless for us without something like what I'm proposing in https://bugs.webkit.org/show_bug.cgi?id=115871 .

[Benjamin Poulain]

(In reply to comment #15)
> So far, Nix needs such viewport information on its API layer so the application can use that information to implement whatever is the way it wants to interact with the page. We don't use the PageViewportController as the other CoordinatedGraphics ports do.
> 
> However, currently WKViewClient is exposing a WKViewportAttribute which is useless for us without something like what I'm proposing in https://bugs.webkit.org/show_bug.cgi?id=115871 .

Let say I were to expose the way Transforms are applied to the API. That way each embedded could decide how to apply matrices to coordinates. Would you say it is a good idea?

This is how I see exposing viewport attribute. The input and computation should not matter for embedders and WebKit should be able to rely on deterministic properties given a certain input. Thus we should exposed the computed results, not a raw input.

[Jesus Sanchez-Palencia]

(In reply to comment #16)
> (In reply to comment #15)
> > So far, Nix needs such viewport information on its API layer so the application can use that information to implement whatever is the way it wants to interact with the page. We don't use the PageViewportController as the other CoordinatedGraphics ports do.
> > 
> > However, currently WKViewClient is exposing a WKViewportAttribute which is useless for us without something like what I'm proposing in https://bugs.webkit.org/show_bug.cgi?id=115871 .
> 
> Let say I were to expose the way Transforms are applied to the API. That way each embedded could decide how to apply matrices to coordinates. Would you say it is a good idea?

Well, funny enough this is something that we also do indeed: https://github.com/WebKitNix/webkitnix/blob/master/Source/WebKit2/UIProcess/API/nix/NIXView.h#L103