Summary: | [WK2][EFL] WebView: Add callbacks to the WKViewClient to handle page viewport update | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail Pozdnyakov <mikhail.pozdnyakov> | ||||||||
Component: | WebKit EFL | Assignee: | Mikhail Pozdnyakov <mikhail.pozdnyakov> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abecsi, andersca, ap, benjamin, cmarcelo, commit-queue, gyuyoung.kim, hugo.lima, jesus, kenneth, kling, lucas.de.marchi, luciano.wolf, noam, rakuco, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 114593 | ||||||||||
Bug Blocks: | 115871, 107657, 107662, 110741, 111543 | ||||||||||
Attachments: |
|
Description
Mikhail Pozdnyakov
2013-02-21 06:25:15 PST
Created attachment 189748 [details]
WIP
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
(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. 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.
Comment on attachment 189765 [details]
patch
LGTM
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? (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. Wait what? What do you think a WebPage is then? And the PageUIClient? (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). Created attachment 197269 [details]
rebased
(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. :) Comment on attachment 197269 [details] rebased Clearing flags on attachment: 197269 Committed r148274: <http://trac.webkit.org/changeset/148274> All reviewed patches have been landed. Closing bug. 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. 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 . (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. (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 |