Bug 73994 – [Qt][WK2] Fix tst_QQuickWebView::scrollRequest() API test

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

FAIL!  : tst_QQuickWebView::scrollRequest() 'webView()->page()->pos().y() == y' returned FALSE. ()
   Loc: [/ramdisk/qt-linux-32-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qquickwebview/tst_qquickwebview.cpp(267)]
QWARN  : tst_QQuickWebView::show() QQuickCanvas: platform does not support threaded rendering!

[Csaba Osztrogonác]

The QWARN line is unrelated to this bug, I pastes it accidentally.

[Kenneth Rohde Christiansen]

Source/WebKit2/UIProcess/API/qt/tests/html/scroll.html is in there twice, weird.

This seems to be related to the suspended check. 

Cc'ing Alex as he looked into that yesterday.

[Michael Brüning]

It already happened before Alex's patch landed (saw it yesterday)...

[Michael Brüning]

I'll take a look at it.

[Michael Brüning]

Created attachment 118401 [details]
Proposed patch

Patch to fix the test case and correct the html for the test.
Reloading test case etc coming up separately.

[Kenneth Rohde Christiansen]

Comment on attachment 118401 [details]
Proposed patch

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

r+ as this is an improvement, but this could be even better

> Source/WebKit2/UIProcess/API/qt/tests/html/scroll.html:26
> +This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page This is a test page 

There are other ways to make the page long.... for instance a div with height set. (color as well would be preferred)

> Source/WebKit2/UIProcess/API/qt/tests/qquickwebview/tst_qquickwebview.cpp:263
> +    QTest::qWait(200);

?This seems more like a workaround. Why is there no comment why this is needed, as it is not obvious?

> Source/WebKit2/UIProcess/API/qt/tests/qquickwebview/tst_qquickwebview.cpp:273
> +    int y = -qRound(50 * webView()->page()->scale());
>      QVERIFY(webView()->page()->pos().y() == y);
>  }

Please add a reload of the page here and verify again that it is at the right place. Can be another patch.

[WebKit Review Bot]

Comment on attachment 118401 [details]
Proposed patch

Clearing flags on attachment: 118401

Committed r102453: <http://trac.webkit.org/changeset/102453>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

Reopen, because this test still fails on the bot.

[Michael Brüning]

Created attachment 120176 [details]
Patch to fix the problem.

Found that the problem was actually in a wrong/missing viewport definition, which cause the page to be scaled and not scrollable in xvfb.
Added a viewport meta tag and found out that parts of the previous patch to the test weren't necessary, hence reverted those (see Source/WebKit2/UIProcess/API/qt/test/qquickwebview/tst_qquickwebview.cpp for details).
Removed unnecessary "This is a test page" text and corrected width and height in the div style definition. 

Tested on host and in xvfb.

[WebKit Review Bot]

Comment on attachment 120176 [details]
Patch to fix the problem.

Clearing flags on attachment: 120176

Committed r103410: <http://trac.webkit.org/changeset/103410>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.