Review Request: flicking of WebView with ScrollWidget

[Zack Rusin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3368/#review4626
-----------------------------------------------------------



/trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp
<http://reviewboard.kde.org/r/3368/#comment4130>

    Why was this needed? If we'll filter all child events then it will be possible to flick the scrollbars. The check should probably either stay as it is or be changed to
    if (!d->widget.data() || i == d->verticalScrollBar || i == d->horizontalScrollBar)
        return false;



/trunk/KDE/kdelibs/plasma/widgets/webview.cpp
<http://reviewboard.kde.org/r/3368/#comment4131>

    You're the biggest event filter fan in the world :) Why do we need that?



/trunk/KDE/kdelibs/plasma/widgets/webview.cpp
<http://reviewboard.kde.org/r/3368/#comment4132>

    I'd probably just create a vertical or horizontal layout, put the d->webView in it and set the layout on the d->scrollingWidget, this way you don't have to worry about setting d->webView geometry. But I'm not 100% sure if we need the scrollWidget at all, wouldn't it be easier to just set ScrollWidget::setWidget(d->webView) directly?


I think ideally we shouldn't need a dummy scrollingWidget or any event filters. If it doesn't work then, unless I'm missing something here, there's probably a bug/missing feature in the Plasma::ScrollWidget. I could look at it tomorrow night.
Either way we should probably test it with the new QtWebKit with the AcceleratedCompositing option on to make sure it works at all.

- Zack


On 2010-03-23 14:54:52, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3368/
> -----------------------------------------------------------
> 
> (Updated 2010-03-23 14:54:52)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> after the last modifications by Zack on Scrollwidget, now flicking is managed by ScrollWidget itself, because it's the only way it can safely filter child events..
> and since html elements will be done by qraphicsitems, it will probably be necessary to do the same thing in WebView.
> this is an early attempt, but doesn't seem to work very well. there is a qgraphicswidget with geometry syncronized with the page contents so flicking it makes the page scroll around.
> 
> it's not always correct since is not possible to watch for a change in the zoom level.
> things still missing: disabling everything with setDragToScroll and support for multitouch events.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1106404 
>   /trunk/KDE/kdelibs/plasma/widgets/webview.h 1106402 
>   /trunk/KDE/kdelibs/plasma/widgets/webview.cpp 1106402 
> 
> Diff: http://reviewboard.kde.org/r/3368/diff
> 
> 
> Testing
> -------
> 
> it seems significantly slower and quite buggy, but i don't see much more ways to do it.
> 
> another way would be generalizing scrollwidget to make it possible to set properties in the webview instead of moving the clipped widget into the parent, but it would make it even more complex.
> 
> another way could be moving everything back in kineticscroll again, make it visible to the classes that are using it (so won't be possible anymore to use it outside of libplasma) and forward by hand to it sceneeventfilter.
> 
> the latter is at the moment the one i'm leaning towards.
> 
> 
> Thanks,
> 
> Marco
> 
>