[webkit-reviews] review denied: [Bug 32668] [Qt] Add new scrollRecursively API to QWebFrame : [Attachment 45081] Patch to add new QWebFrame API scrollRecursively
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Dec 17 10:17:16 PST 2009
Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Joseph Ligman
<joseph.ligman at nokia.com>'s request for review:
Bug 32668: [Qt] Add new scrollRecursively API to QWebFrame
https://bugs.webkit.org/show_bug.cgi?id=32668
Attachment 45081: Patch to add new QWebFrame API scrollRecursively
https://bugs.webkit.org/attachment.cgi?id=45081&action=review
------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
> Index: WebKit/qt/ChangeLog
> ===================================================================
> --- WebKit/qt/ChangeLog (revision 52259)
> +++ WebKit/qt/ChangeLog (working copy)
> @@ -1,3 +1,21 @@
> +2009-12-17 Joe Ligman <joseph.ligman at nokia.com>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + [Qt] Add new API to QWebFrame to scrollRecursively starting with any
css overflow
> + then checking current frame and then ancestors
> + https://bugs.webkit.org/show_bug.cgi?id=32668
> +
> + * Api/qwebframe.cpp:
> + (QWebFramePrivate::scrollOverflow):
> + (QWebFrame::scrollRecursively):
> + * Api/qwebframe.h:
> + * Api/qwebframe_p.h:
> + * tests/qwebframe/qwebframe.qrc:
> + * tests/qwebframe/testiframe.html: Added.
> + * tests/qwebframe/testiframe2.html: Added.
> + * tests/qwebframe/tst_qwebframe.cpp:
> +
> 2009-12-17 Benjamin Poulain <benjamin.poulain at nokia.com>
>
> Reviewed by Simon Hausmann.
> Index: WebKit/qt/Api/qwebframe.cpp
> ===================================================================
> --- WebKit/qt/Api/qwebframe.cpp (revision 52259)
> +++ WebKit/qt/Api/qwebframe.cpp (working copy)
> @@ -361,6 +361,45 @@ void QWebFramePrivate::renderRelativeCoo
> }
> }
>
> +bool QWebFramePrivate::scrollOverflow(int dx, int dy)
> +{
> + if (!frame || !frame->document() || !frame->eventHandler())
> + return false;
> +
> + Node* node = frame->document()->focusedNode();
> + if (!node)
> + node =
frame->document()->elementFromPoint(frame->eventHandler()->currentMousePosition
().x(),
> +
frame->eventHandler()->currentMousePosition().y());
Remove the space before frame->
> + if (!node)
> + return false;
> +
> + RenderObject* renderer = node->renderer();
> + if (!renderer)
> + return false;
> +
> + if (renderer->isListBox())
> + return false;
> +
> + RenderLayer* renderLayer = renderer->enclosingLayer();
> + if (!renderLayer)
> + return false;
> +
> + bool scrolledHorizontal = false;
> + bool scrolledVertical = false;
> +
> + if (dx > 0)
> + scrolledHorizontal = renderLayer->scroll(ScrollRight, ScrollByPixel,
dx);
> + else if (dx < 0)
> + scrolledHorizontal = renderLayer->scroll(ScrollLeft, ScrollByPixel,
qAbs(dx));
> +
> + if (dy > 0)
> + scrolledVertical = renderLayer->scroll(ScrollDown, ScrollByPixel,
dy);
> + else if (dy < 0)
> + scrolledVertical = renderLayer->scroll(ScrollUp, ScrollByPixel,
qAbs(dy));
> +
> + return (scrolledHorizontal || scrolledVertical);
> +}
> +
> /*!
> \class QWebFrame
> \since 4.4
> @@ -1000,6 +1039,50 @@ void QWebFrame::scroll(int dx, int dy)
> }
>
> /*!
> + \since 4.7
> + Scrolls nested frames starting at this frame, \a dx pixels to the right
> + and \a dy pixels downward. Both \a dx and \a dy may be negative. First
attempts
> + to scroll elements with css overflow followed by this frame. If this
It is called CSS
> + frame doesn't scroll, attempts to scroll the parent
> +
> + \sa QWebFrame::scroll, QWebFramePrivate::scrollOverflow
Dont add references to the private!
> +*/
> +bool QWebFrame::scrollRecursively(int dx, int dy)
> +{
> + bool scrolledHorizontal = false;
> + bool scrolledVertical = false;
> + bool scrolledOverflow = d->scrollOverflow(dx, dy);
> +
> + if (!scrolledOverflow) {
> + Frame* frame = d->frame;
> + if (!frame || !frame->view())
> + return false;
> +
> + do {
> + IntSize scrollOffset = frame->view()->scrollOffset();
> + IntPoint maxScrollOffset =
frame->view()->maximumScrollPosition();
> +
> + if (dx > 0) // scroll right
> + scrolledHorizontal = scrollOffset.width() <
maxScrollOffset.x();
> + else if (dx < 0) // scroll left
> + scrolledHorizontal = scrollOffset.width() > 0;
> +
> + if (dy > 0) // scroll down
> + scrolledVertical = scrollOffset.height() <
maxScrollOffset.y();
> + else if (dy < 0) //scroll up
> + scrolledVertical = scrollOffset.height() > 0;
> +
> + if (scrolledHorizontal || scrolledVertical) {
> + frame->view()->scrollBy(IntSize(dx, dy));
> + return true;
> + }
> + frame = frame->tree()->parent();
> + } while (frame && frame->view());
> + }
> + return (scrolledHorizontal || scrolledVertical || scrolledOverflow);
> +}
> +
> +/*!
> \property QWebFrame::scrollPosition
> \since 4.5
> \brief the position the frame is currently scrolled to.
> Index: WebKit/qt/Api/qwebframe.h
> ===================================================================
> --- WebKit/qt/Api/qwebframe.h (revision 52259)
> +++ WebKit/qt/Api/qwebframe.h (working copy)
> @@ -156,6 +156,7 @@ public:
> QRect scrollBarGeometry(Qt::Orientation orientation) const;
>
> void scroll(int, int);
> + bool scrollRecursively(int, int);
> QPoint scrollPosition() const;
> void setScrollPosition(const QPoint &pos);
>
> Index: WebKit/qt/Api/qwebframe_p.h
> ===================================================================
> --- WebKit/qt/Api/qwebframe_p.h (revision 52259)
> +++ WebKit/qt/Api/qwebframe_p.h (working copy)
> @@ -85,6 +85,8 @@ public:
> void renderRelativeCoords(WebCore::GraphicsContext*,
QWebFrame::RenderLayer, const QRegion& clip);
> void renderContentsLayerAbsoluteCoords(WebCore::GraphicsContext*, const
QRegion& clip);
>
> + bool scrollOverflow(int dx, int dy);
> +
> QWebFrame *q;
> Qt::ScrollBarPolicy horizontalScrollBarPolicy;
> Qt::ScrollBarPolicy verticalScrollBarPolicy;
> Index: WebKit/qt/tests/qwebframe/qwebframe.qrc
> ===================================================================
> --- WebKit/qt/tests/qwebframe/qwebframe.qrc (revision 52259)
> +++ WebKit/qt/tests/qwebframe/qwebframe.qrc (working copy)
> @@ -4,5 +4,7 @@
> <file>style.css</file>
> <file>test1.html</file>
> <file>test2.html</file>
> +<file>testiframe.html</file>
> +<file>testiframe2.html</file>
> </qresource>
> </RCC>
> Index: WebKit/qt/tests/qwebframe/testiframe2.html
> ===================================================================
> --- WebKit/qt/tests/qwebframe/testiframe2.html (revision 0)
> +++ WebKit/qt/tests/qwebframe/testiframe2.html (revision 0)
> @@ -0,0 +1,21 @@
> +</html>
> +<html>
> +<head>
> +<title></title>
> +<style type="text/css">
> +<!--
> +#content {
> + background: #fff;
> + position: absolute;
> + top: 0px;
> + left: 0px;
> + width: 800px;
> + height: 800px;
> +}
> +-->
> +</style>
> +</head>
> +<body>
> +<div id="content"> </div>
> +</body>
> +</html>
> \ No newline at end of file
> Index: WebKit/qt/tests/qwebframe/testiframe.html
> ===================================================================
> --- WebKit/qt/tests/qwebframe/testiframe.html (revision 0)
> +++ WebKit/qt/tests/qwebframe/testiframe.html (revision 0)
> @@ -0,0 +1,54 @@
> +</html>
> +<html>
> +<head>
> +<title></title>
> +<style type="text/css">
> +<!--
> +#header {
> + background: #0f0;
> + position: absolute;
> + top: 0px;
> + left: 0px;
> + width: 800px;
> + height: 100px;
> +}
> +#content1 {
> + background: #ff0;
> + position: absolute;
> + top: 101px;
> + left: 0px;
> + width: 400px;
> + height: 400px;
> + overflow: scroll;
> +}
> +#content2 {
> + background: #ff7;
> + position: absolute;
> + top: 101px;
> + left: 401px;
> + width: 400px;
> + height: 400px;
> +}
> +#footer {
> + background: #0f0;
> + position: absolute;
> + top: 502px;
> + left: 0px;
> + width: 800px;
> + height: 200px;
> +}
> +-->
> +</style>
> +</head>
> +<body>
> +<div id="header"></div>
> +<div id="content1">You can use the overflow property when you want to have
better control of the layout. Try to change the overflow property to: visible,
hidden, auto, or inherit and see what happens. The default value is visible.
> +You can use the overflow property when you want to have better control of
the layout. Try to change the overflow property to: visible, hidden, auto, or
inherit and see what happens. The default value is visible.
> +You can use the overflow property when you want to have better control of
the layout. Try to change the overflow property to: visible, hidden, auto, or
inherit and see what happens. The default value is visible.
> +You can use the overflow property when you want to have better control of
the layout. Try to change the overflow property to: visible, hidden, auto, or
inherit and see what happens. The default value is visible.
> +You can use the overflow property when you want to have better control of
the layout. Try to change the overflow property to: visible, hidden, auto, or
inherit and see what happens. The default value is visible.
> +You can use the overflow property when you want to have better control of
the layout. Try to change the overflow property to: visible, hidden, auto, or
inherit and see what happens. The default value is visible.</div>
> +<iframe id="content2" name="control" src="testiframe2.html"> </iframe>
> +<div id="footer"></div>
> +</body>
> +</html>
> \ No newline at end of file
> Index: WebKit/qt/tests/qwebframe/tst_qwebframe.cpp
> ===================================================================
> --- WebKit/qt/tests/qwebframe/tst_qwebframe.cpp (revision 52259)
> +++ WebKit/qt/tests/qwebframe/tst_qwebframe.cpp (working copy)
> @@ -576,6 +576,7 @@ private slots:
> void scrollPosition();
> void evaluateWillCauseRepaint();
> void qObjectWrapperWithSameIdentity();
> + void scrollRecursively();
>
> private:
> QString evalJS(const QString&s) {
> @@ -2795,5 +2796,69 @@ void tst_QWebFrame::qObjectWrapperWithSa
> QCOMPARE(mainFrame->toPlainText(), QString("test2"));
> }
>
> +void tst_QWebFrame::scrollRecursively()
> +{
> + // The test content is
> + // a nested frame set
> + // The main frame scrolls
> + // and has two children
> + // an iframe and a div overflow
> + // both scroll
> + QWebView webView;
> + QWebPage* webPage = webView.page();
> + QSignalSpy loadSpy(webPage, SIGNAL(loadFinished(bool)));
> + QUrl url = QUrl("qrc:///testiframe.html");
> + webPage->mainFrame()->load(url);
> + QTRY_COMPARE(loadSpy.count(), 1);
> +
> + QList<QWebFrame*> children = webPage->mainFrame()->childFrames();
> + QVERIFY(children.count() == 1);
> +
> + // 1st test
> + // call scrollRecursively over mainframe
> + // verify scrolled
> + // verify scroll postion changed
> + QPoint scrollPosition(webPage->mainFrame()->scrollPosition());
> + QVERIFY(webPage->mainFrame()->scrollRecursively(10, 10));
> + QVERIFY(scrollPosition != webPage->mainFrame()->scrollPosition());
> +
> + // 2nd test
> + // call scrollRecursively over child iframe
> + // verify scrolled
> + // verify child scroll position changed
> + // verify parent's scroll position did not change
> + scrollPosition = webPage->mainFrame()->scrollPosition();
> + QPoint childScrollPosition = children.at(0)->scrollPosition();
> + QVERIFY(children.at(0)->scrollRecursively(10, 10));
> + QVERIFY(scrollPosition == webPage->mainFrame()->scrollPosition());
> + QVERIFY(childScrollPosition != children.at(0)->scrollPosition());
> +
> + // 3rd test
> + // call scrollRecursively over div overflow
> + // verify scrolled == true
> + // verify parent and child frame's scroll postion did not change
> + QWebElement div =
webPage->mainFrame()->documentElement().findFirst("#content1");
> + QMouseEvent evpres(QEvent::MouseMove, div.geometry().center(),
Qt::NoButton, Qt::NoButton, Qt::NoModifier);
> + webPage->event(&evpres);
> + scrollPosition = webPage->mainFrame()->scrollPosition();
> + childScrollPosition = children.at(0)->scrollPosition();
> + QVERIFY(webPage->mainFrame()->scrollRecursively(5, 5));
> + QVERIFY(childScrollPosition == children.at(0)->scrollPosition());
> + QVERIFY(scrollPosition == webPage->mainFrame()->scrollPosition());
> +
> + // 4th test
> + // call scrollRecursively twice over childs iframe
> + // verify scrolled == true first time
> + // verify parent's scroll == true second time
> + // verify parent and childs scroll position changed
> + childScrollPosition = children.at(0)->scrollPosition();
> + QVERIFY(children.at(0)->scrollRecursively(-10, -10));
> + QVERIFY(childScrollPosition != children.at(0)->scrollPosition());
> + scrollPosition = webPage->mainFrame()->scrollPosition();
> + QVERIFY(children.at(0)->scrollRecursively(-10, -10));
> + QVERIFY(scrollPosition != webPage->mainFrame()->scrollPosition());
> +
> +}
> +
> QTEST_MAIN(tst_QWebFrame)
> #include "tst_qwebframe.moc"
More information about the webkit-reviews
mailing list