[Webkit-unassigned] [Bug 34267] [Qt] Support kinetic scrolling on Maemo 5

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=34267


Kenneth Rohde Christiansen <kenneth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #47620|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #2 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-01-28 08:22:39 PST ---
(From update of attachment 47620)
> diff --git a/WebKit/qt/Api/qwebview.cpp b/WebKit/qt/Api/qwebview.cpp
> index 9ef2e55..4eeb8ad 100644
> --- a/WebKit/qt/Api/qwebview.cpp
> +++ b/WebKit/qt/Api/qwebview.cpp
> @@ -58,6 +58,107 @@ void QWebViewPrivate::_q_pageDestroyed()
>      view->setPage(0);
>  }
>  
> +#ifdef Q_WS_MAEMO_5
> +#include "qabstractkineticscroller.h"
> +
> +class QWebViewKineticScroller : public QAbstractKineticScroller {
> +public:
> +    QWebViewKineticScroller()
> +        : QAbstractKineticScroller()
> +    {}

I would put the above on one line, since we have no implementation anyway

> +
> +    // remember the frame where the button was pressed
> +    bool eventFilter(QObject *o, QEvent *e)

Almost all of qwebview.cpp uses ev and not e, please change

> +    {
> +        switch (e->type()) {
> +        case QEvent::MouseButtonPress: {
> +            QWebFrame *hitFrame = scrollingFrameAt(static_cast<QMouseEvent *>(e)->pos());

coding style, * should be aligned to the left

> +            if (hitFrame)
> +                _frame = hitFrame;

please use m_frame, which is webkit style.

> +            break;
> +        }
> +        default:
> +            break;
> +        }
> +        return QAbstractKineticScroller::eventFilter(o, e);
> +    }
> +
> +protected:
> +    QWebFrame *currentFrame() const

here as well

> +    {
> +        if (!_frame.isNull())
> +            return _frame.data();
> +
> +        QWebView *web = static_cast<QWebView *>(widget());

web is a bad variable name, maybe use view instead

> +        QWebFrame *frame = web->page()->mainFrame();

* again, please also run check-webkit-style on it

> +        return frame;
> +    }
> +
> +    /*! Returns the innermost frame at the given position that can scroll. */

use // commenting

> +    QWebFrame *scrollingFrameAt(const QPoint &pos) const

coding style, * and &

> +    {
> +        QWebView *web = static_cast<QWebView *>(widget());

view

> +        QWebFrame *mainFrame = web->page()->mainFrame();
> +        QWebFrame *hitFrame = mainFrame->hitTestContent(pos).frame();

coding style

> +        QSize range = hitFrame->contentsSize() - hitFrame->geometry().size();
> +
> +        while (hitFrame && range.width() <= 1 && range.height() <= 1)
> +            hitFrame = hitFrame->parentFrame();

Are you sure infinite loops can never occour here?

> +
> +        return hitFrame;
> +    }
> +
> +    void attachToWidget()
> +    {
> +        QWebView *web = static_cast<QWebView *>(widget());
> +        QWebFrame *mainFrame = web->page()->mainFrame();
> +        mainFrame->setScrollBarPolicy(Qt::Vertical, Qt::ScrollBarAlwaysOff);
> +        mainFrame->setScrollBarPolicy(Qt::Horizontal, Qt::ScrollBarAlwaysOff);
> +        web->installEventFilter(this);
> +    }
> +
> +    void removeFromWidget()
> +    {
> +        QWebView *web = static_cast<QWebView *>(widget());
> +        web->removeEventFilter(this);
> +        QWebFrame *mainFrame = web->page()->mainFrame();
> +        mainFrame->setScrollBarPolicy(Qt::Vertical, Qt::ScrollBarAsNeeded);
> +        mainFrame->setScrollBarPolicy(Qt::Horizontal, Qt::ScrollBarAsNeeded);

Shouldn't you save the old scrollbar policyt instead and reapply it?

> +    }
> +
> +    QRect positionRange() const
> +    {
> +        QRect r;
> +        QWebFrame *frame = currentFrame();

style

> +        r.setSize(frame->contentsSize() - frame->geometry().size());
> +        return r;
> +    }
> +
> +    QPoint position() const
> +    {
> +        QWebFrame *frame = currentFrame();

style

> +        return frame->scrollPosition();
> +    }
> +
> +    QSize viewportSize() const
> +    {
> +        return static_cast<QWebView *>(widget())->page()->viewportSize();

style.

> +    }
> +
> +    void setPosition(const QPoint &p, const QPoint &overShootDelta)
> +    {
> +        Q_UNUSED(overShootDelta);

not webkit style, just leave out the second param. (please use point as well,
not p)

or do

void setPosition(const QPoint& point, const QPoint& /* overShootDelta */)

> +
> +        QWebFrame *frame = currentFrame();
> +        frame->setScrollPosition(p);
> +    }
> +
> +    QPointer<QWebFrame> _frame;
> +};
> +
> +#endif // Q_WS_MAEMO_5
> +
> +
>  /*!
>      \class QWebView
>      \since 4.4
> @@ -157,7 +258,11 @@ QWebView::QWebView(QWidget *parent)
>  #if QT_VERSION >= QT_VERSION_CHECK(4, 6, 0)
>      setAttribute(Qt::WA_AcceptTouchEvents);
>  #endif
> -
> +#if defined(Q_WS_MAEMO_5)
> +    QAbstractKineticScroller *ks = new QWebViewKineticScroller();

use scroller instead of ks, *

> +    ks->setWidget(this);
> +    setProperty("kineticScroller", QVariant::fromValue(ks));
> +#endif
>      setAcceptDrops(true);
>  
>      setMouseTracking(true);
> diff --git a/WebKit/qt/ChangeLog b/WebKit/qt/ChangeLog
> index f9aa123..f295572 100644
> --- a/WebKit/qt/ChangeLog
> +++ b/WebKit/qt/ChangeLog
> @@ -1,3 +1,27 @@
> +2010-01-28  Andreas Kling  <andreas.kling at nokia.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [Qt] Support kinetic scrolling on Maemo 5
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=34267
> +
> +        Patch by Ralf Engels <ralf.engels at nokia.com> and
> +        Robert Griebl <rgriebl at trolltech.com>
> +
> +        * Api/qwebview.cpp:
> +        (QWebViewKineticScroller::QWebViewKineticScroller):
> +        (QWebViewKineticScroller::eventFilter):
> +        (QWebViewKineticScroller::currentFrame):
> +        (QWebViewKineticScroller::scrollingFrameAt):
> +        (QWebViewKineticScroller::attachToWidget):
> +        (QWebViewKineticScroller::removeFromWidget):
> +        (QWebViewKineticScroller::positionRange):
> +        (QWebViewKineticScroller::position):
> +        (QWebViewKineticScroller::viewportSize):
> +        (QWebViewKineticScroller::setPosition):
> +        (QWebView::QWebView):
> +
>  2010-01-28  Kenneth Rohde Christiansen  <kenneth at webkit.org>
>  
>          Reviewed by Simon Hausmann.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.