Bug 88531 - [Qt][WK2] Fix custom device pixel ratio propagation and add QML API tests
Summary: [Qt][WK2] Fix custom device pixel ratio propagation and add QML API tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andras Becsi
URL:
Keywords:
Depends on: 88870
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-07 06:37 PDT by Alexander Færøy
Modified: 2012-08-24 06:50 PDT (History)
8 users (show)

See Also:


Attachments
Patch (2.48 KB, patch)
2012-06-07 06:43 PDT, Alexander Færøy
no flags Details | Formatted Diff | Diff
Patch (3.17 KB, patch)
2012-06-07 07:36 PDT, Alexander Færøy
no flags Details | Formatted Diff | Diff
proposed patch (9.44 KB, patch)
2012-08-23 11:36 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff
proposed patch (9.67 KB, patch)
2012-08-24 04:24 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Færøy 2012-06-07 06:37:29 PDT
SSIA.
Comment 1 Alexander Færøy 2012-06-07 06:43:47 PDT
Created attachment 146278 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-06-07 06:51:17 PDT
Comment on attachment 146278 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml:32
> +            webView.experimental.devicePixelRatio = 0.2

Why not just use 2.0

> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml:35
> +            webView.experimental.evaluateJavaScript(

Why no test for the media query?

> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml:36
> +                "(function() { return window.devicePixelRatio })()",

isnt this called webkitDevicePixelRatio?
Comment 3 Alexander Færøy 2012-06-07 07:36:28 PDT
Created attachment 146288 [details]
Patch
Comment 4 Csaba Osztrogonác 2012-06-08 03:49:01 PDT
Landed in http://trac.webkit.org/changeset/119723,
but there are two failing API tests:

XPASS  : qmltests::DevicePixelRatio::test_devicePixelRatio() '' returned TRUE unexpectedly. ()
   Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml(33)]
XPASS  : qmltests::DevicePixelRatio::test_devicePixelRatioMediaQuery() '' returned TRUE unexpectedly. ()
   Loc: [/home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml(51)]

Could you check it, please?
Comment 5 Csaba Osztrogonác 2012-06-19 08:36:34 PDT
The patch landed, so we should close it ... But don't forget that it is incorrect - https://bugs.webkit.org/show_bug.cgi?id=88870
Comment 6 Csaba Osztrogonác 2012-07-31 01:43:31 PDT
(In reply to comment #5)
> The patch landed, so we should close it ... But don't forget that it is incorrect - https://bugs.webkit.org/show_bug.cgi?id=88870

Guys, please answer and don't ignore this bug ...
Comment 7 Csaba Osztrogonác 2012-08-07 14:54:24 PDT
Reopen, because it was rolled out by http://trac.webkit.org/changeset/124923.
See https://bugs.webkit.org/show_bug.cgi?id=88870 for details.
Comment 8 Andras Becsi 2012-08-23 11:36:50 PDT
Created attachment 160203 [details]
proposed patch
Comment 9 Kenneth Rohde Christiansen 2012-08-23 11:44:42 PDT
Comment on attachment 160203 [details]
proposed patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:803
> +    // Set the custom device pixel ratio requested from QML
> +    // as soon as the content item has a valid size.
> +    webPageProxy->setCustomDeviceScaleFactor(m_customDevicePixelRatio);

It ignores it if not? Why does it do that?
Comment 10 Andras Becsi 2012-08-23 13:52:46 PDT
(In reply to comment #9)
> (From update of attachment 160203 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=160203&action=review
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:803
> > +    // Set the custom device pixel ratio requested from QML
> > +    // as soon as the content item has a valid size.
> > +    webPageProxy->setCustomDeviceScaleFactor(m_customDevicePixelRatio);
> 
> It ignores it if not? Why does it do that?

The DrawingAreaProxy returns early if the page size is empty and the time when the experimental property is set the QML page item has no valid size yet thus the information does not reach the web process.
Comment 11 Kenneth Rohde Christiansen 2012-08-23 14:06:28 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 160203 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=160203&action=review
> > 
> > > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:803
> > > +    // Set the custom device pixel ratio requested from QML
> > > +    // as soon as the content item has a valid size.
> > > +    webPageProxy->setCustomDeviceScaleFactor(m_customDevicePixelRatio);
> > 
> > It ignores it if not? Why does it do that?
> 
> The DrawingAreaProxy returns early if the page size is empty and the time when the experimental property is set the QML page item has no valid size yet thus the information does not reach the web process.

I think that comment there would be quite valuable.
Comment 12 Andras Becsi 2012-08-24 04:24:45 PDT
Created attachment 160392 [details]
proposed patch

With additional comment explaining the issue.
Comment 13 Andras Becsi 2012-08-24 06:50:22 PDT
Comment on attachment 160392 [details]
proposed patch

Clearing flags on attachment: 160392

Committed r126582: <http://trac.webkit.org/changeset/126582>
Comment 14 Andras Becsi 2012-08-24 06:50:27 PDT
All reviewed patches have been landed.  Closing bug.