[Qt][WK2] Fix painting on Mac with retina display

• Component: New Bugs
• Status: RESOLVED FIXED

[Andras Becsi]

Created attachment 178588 [details]
MiniBrowser scaling issue on retina display Mac

Since support for devicePixelRatio (HiDPI) has been added to qtbase (in 5e61bbe586519c3d9bc636153d32e810da4e59a3) and subsequently to qtdeclarative (in 9c9ad9b86f5ae30df1021300d1b775f6d341a78f) the page contents are painted incorrectly in the WebView on a Mac with a retina display (see attached image).

Patch is on the way.

[Andras Becsi]

Created attachment 178602 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 178602 [details]
Patch

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

> Source/WebKit2/ChangeLog:9
> +        painting inclorrectly scaled content on high-resolution screens.

spelling

> Source/WebKit2/ChangeLog:13
> +        Because the intrinsic device pixel ratio is always taken into
> +        account by Qt when painting to high-resolution screens we should
> +        automatically obtain the scale ratio from the window in which the
> +        item is rendered instead of setting it in QML.

Just for my curiosity, what if I do a mobile browser for a platform. How can I tell Qt to apply some kind of scaling?

> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml:-55
> -            verify(webView.waitForLoadSucceeded())
> -
> -            webView.experimental.evaluateJavaScript(
> -                "(function() { return window.matchMedia(\"(-webkit-device-pixel-ratio: 2)\").matches })()",
> -                function(result) {
> -                    webView.lastResult = result

I still think it is worth having this tested somehow

[Andras Becsi]

(In reply to comment #2)
> > Source/WebKit2/ChangeLog:13
> > +        Because the intrinsic device pixel ratio is always taken into
> > +        account by Qt when painting to high-resolution screens we should
> > +        automatically obtain the scale ratio from the window in which the
> > +        item is rendered instead of setting it in QML.
> 
> Just for my curiosity, what if I do a mobile browser for a platform. How can I tell Qt to apply some kind of scaling?

The support is quite basic in Qt yet, but you can set the device pixel ratio on QPixmaps and QImages.
I'm not familiar with the specifics but I think a platform-specific device pixel ratio for a conceptual device could be specifies with reimplementing QPlatformScreen::devicePixelRatio() / QPlatformWindow::devicePixelRatio(), but I think this is not feasible for us at this point.

We might get more comprehensive support with the Android and iOS ports later, also for simulating device pixel ratio, but until then I think it is best to make it work on real life hardware instead of a conceptual one, without breaking on normal resolution displays, and that's what the patch gives a solution for.

> 
> > Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml:-55
> > -            verify(webView.waitForLoadSucceeded())
> > -
> > -            webView.experimental.evaluateJavaScript(
> > -                "(function() { return window.matchMedia(\"(-webkit-device-pixel-ratio: 2)\").matches })()",
> > -                function(result) {
> > -                    webView.lastResult = result
> 
> I still think it is worth having this tested somehow

Well, I could move these to QWebKitTest since they test the propagation of the device pixel ratio value to JavaScript and not painting, but without an actual Qt API it would only test platform independent code which I think would not give us much after al.

[Simon Hausmann]

Comment on attachment 178602 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:91
> +    if (const QQuickWindow* const w = window()) {

I think the type required for devicePixelRatio() is only QWindow.

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:94
> +            d->webPageProxy->setIntrinsicDeviceScaleFactor(devicePixelRatio);

Since the ratio seems to be a constant at this point, why not set it earlier? (as soon as we have a QWindow, i.e. in canvasChanged)

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:95
> +            node->setDevicePixelRatio(devicePixelRatio);

Hmm, why copy the value here instead of just reading it from the window when needed?

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:96
> +            emit d->viewportItem->experimental()->test()->devicePixelRatioChanged();

If you removed the signal, why emit it here? :)

>> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml:-55
>> -                    webView.lastResult = result
> 
> I still think it is worth having this tested somehow

This seems like something that should be tested via layout tests, no? Picking up a value from the platform (or configurable via LayoutTestController) and propagating it correctly through the engine into the web facing APIs. I don't think we need a QML test here :)

[Simon Hausmann]

Just to clarify: I love this direction. It seems indeed that we should not have any API for this but just pick up the right value from the platform. As long as QWindow::devicePixelRatio() is indeed the same as window.devicePixelRatio in JS, this seems like a wonderful match. (Might be worth discussing this with Morten, to be sure the two are in sync)

[Andras Becsi]

(In reply to comment #4)
> (From update of attachment 178602 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178602&action=review
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:91
> > +    if (const QQuickWindow* const w = window()) {
> 
> I think the type required for devicePixelRatio() is only QWindow.
True.
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:94
> > +            d->webPageProxy->setIntrinsicDeviceScaleFactor(devicePixelRatio);
> 
> Since the ratio seems to be a constant at this point, why not set it earlier? (as soon as we have a QWindow, i.e. in canvasChanged)
Hmm, is there such a notification? There does not seem to be a canvasChanged, nor a windowChanged or sceneChanged that would be useful for this purpose. And I think parentChanged does not guarantee the existence of a valid QWindow pointer from window() either.

There might be a mechanism I do not know about.

> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:95
> > +            node->setDevicePixelRatio(devicePixelRatio);
> 
> Hmm, why copy the value here instead of just reading it from the window when needed?
>

Sure, I can pass a QQuickWebPage pointer to the QtWebPageSGNode or a direct pointer to the QWindow but givent the above it seemed to be safer and simpler this way.

> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:96
> > +            emit d->viewportItem->experimental()->test()->devicePixelRatioChanged();
> 
> If you removed the signal, why emit it here? :)

I did not remove the signal from QWebKitTest ;) 
It is still used in the ViewportInfo item of the MiniBrowser that needs notification about the change.

> 
> >> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml:-55
> >> -                    webView.lastResult = result
> > 
> > I still think it is worth having this tested somehow
> 
> This seems like something that should be tested via layout tests, no? Picking up a value from the platform (or configurable via LayoutTestController) and propagating it correctly through the engine into the web facing APIs. I don't think we need a QML test here :)

Yes, it should be possible to construct a test case for that, although I presumed that there already are tests covering this, which is apparently not the case.

[Simon Hausmann]

Comment on attachment 178602 [details]
Patch

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

>>> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:94
>>> +            d->webPageProxy->setIntrinsicDeviceScaleFactor(devicePixelRatio);
>> 
>> Since the ratio seems to be a constant at this point, why not set it earlier? (as soon as we have a QWindow, i.e. in canvasChanged)
> 
> Hmm, is there such a notification? There does not seem to be a canvasChanged, nor a windowChanged or sceneChanged that would be useful for this purpose. And I think parentChanged does not guarantee the existence of a valid QWindow pointer from window() either.
> 
> There might be a mechanism I do not know about.

Looked it up again, the notification I had in mind is

QQuickItem::itemChange(ItemChange change, const ItemChangeData &value)

when

    change == QQuickItem::ItemSceneChange

and the new Q(Quick)Window that is now effective for the item is in 

    value.window

>>>> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml:-55
>>>> -                    webView.lastResult = result
>>> 
>>> I still think it is worth having this tested somehow
>> 
>> This seems like something that should be tested via layout tests, no? Picking up a value from the platform (or configurable via LayoutTestController) and propagating it correctly through the engine into the web facing APIs. I don't think we need a QML test here :)
> 
> Yes, it should be possible to construct a test case for that, although I presumed that there already are tests covering this, which is apparently not the case.

Probably something for a follow-up change for the benefit of all ports supporting DPR.

[Andras Becsi]

(In reply to comment #7)
> (From update of attachment 178602 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178602&action=review
> 
> >>> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:94
> >>> +            d->webPageProxy->setIntrinsicDeviceScaleFactor(devicePixelRatio);
> >> 
> >> Since the ratio seems to be a constant at this point, why not set it earlier? (as soon as we have a QWindow, i.e. in canvasChanged)
> > 
> > Hmm, is there such a notification? There does not seem to be a canvasChanged, nor a windowChanged or sceneChanged that would be useful for this purpose. And I think parentChanged does not guarantee the existence of a valid QWindow pointer from window() either.
> > 
> > There might be a mechanism I do not know about.
> 
> Looked it up again, the notification I had in mind is
> 
> QQuickItem::itemChange(ItemChange change, const ItemChangeData &value)
> 
> when
> 
>     change == QQuickItem::ItemSceneChange
> 
> and the new Q(Quick)Window that is now effective for the item is in 
> 
>     value.window

Thanks. This does not seem to be documented, is that on pupose? :)

> 
> >>>> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_devicePixelRatio.qml:-55
> >>>> -                    webView.lastResult = result
> >>> 
> >>> I still think it is worth having this tested somehow
> >> 
> >> This seems like something that should be tested via layout tests, no? Picking up a value from the platform (or configurable via LayoutTestController) and propagating it correctly through the engine into the web facing APIs. I don't think we need a QML test here :)
> > 
> > Yes, it should be possible to construct a test case for that, although I presumed that there already are tests covering this, which is apparently not the case.
> 
> Probably something for a follow-up change for the benefit of all ports supporting DPR.

Agreed.

[Jocelyn Turcotte]

Comment on attachment 178602 [details]
Patch

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

>> Source/WebKit2/ChangeLog:13
>> +        item is rendered instead of setting it in QML.
> 
> Just for my curiosity, what if I do a mobile browser for a platform. How can I tell Qt to apply some kind of scaling?

It's true that this patch is mixing together the two uses of the pixel ratio:
1. A pixel on the screen is more than one "user pixel" because the OS abstracts it
2. A pixel on the screen is more than one "css pixel" because the page was designed for a screen further from the eye (but the rest of the applications aren't).

It is using a mechanism we had in place for (2.) in order to implement (1.). The problem is that I'm not sure if we can still using it for (2.) without the magic put in place by MacOS.
e.g. A device shipping a browser might want to override the CSS-pixel ratio but still keep the application pixel ratio 1:1.

>>> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:95
>>> +            node->setDevicePixelRatio(devicePixelRatio);
>> 
>> Hmm, why copy the value here instead of just reading it from the window when needed?
> 
> Sure, I can pass a QQuickWebPage pointer to the QtWebPageSGNode or a direct pointer to the QWindow but givent the above it seemed to be safer and simpler this way.

This would be better since the node will get destroyed when the QWindow is hidden.

[Andras Becsi]

Created attachment 178898 [details]
Patch

[Andras Becsi]

(In reply to comment #7)
> (From update of attachment 178602 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178602&action=review
> 
> >>> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:94
> >>> +            d->webPageProxy->setIntrinsicDeviceScaleFactor(devicePixelRatio);
> >> 
> >> Since the ratio seems to be a constant at this point, why not set it earlier? (as soon as we have a QWindow, i.e. in canvasChanged)
> > 
> > Hmm, is there such a notification? There does not seem to be a canvasChanged, nor a windowChanged or sceneChanged that would be useful for this purpose. And I think parentChanged does not guarantee the existence of a valid QWindow pointer from window() either.
> > 
> > There might be a mechanism I do not know about.
> 
> Looked it up again, the notification I had in mind is
> 
> QQuickItem::itemChange(ItemChange change, const ItemChangeData &value)
> 
> when
> 
>     change == QQuickItem::ItemSceneChange
> 
> and the new Q(Quick)Window that is now effective for the item is in 
> 
>     value.window

Unfortunately we can not use this, because the underlying platformWindow has not been created yet by the time QQuickWebPage receives this notification, thus the returned device pixel ratio here is still 1 here.

The platformWindow seems to be created in QWindow::setVisible, thus if we would emit visibleChanged after creating the platformWindow we could connect to that in QQuickItem::itemChange.
I'm not sure if there is a specific reason for the delayed creation though, but patching qtbase for this and connecting to QWindow::visibleChanged does not seem to be worth it.

[Jocelyn Turcotte]

Comment on attachment 178898 [details]
Patch

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

> Source/WebKit2/UIProcess/PageViewportController.cpp:264
> -    drawingArea->setVisibleContentsRect(visibleContentsRect, m_effectiveScale, trajectoryVector);
> +    drawingArea->setVisibleContentsRect(visibleContentsRect, devicePixelRatio() * m_effectiveScale, trajectoryVector);

Following our discussion on IRC, I think that this is going to break the viewport meta tag stuff (i.e. if EFL wants to use it with an explicit parameter like we did before this patch).
Ideally, we would have a way to ask the web process to only scale the backing store resolution and not affect the layout size to avoid having to do this.

> Source/WebKit2/UIProcess/qt/QtWebPageSGNode.cpp:87
>              if (clip->matrix())
>                  clipMatrix = *clip->matrix();
> +            clipMatrix.scale(m_window->devicePixelRatio());

Is it right to scale after the multiplication?
If the clip matrix has a translation this might not give you the final transform you want.
I'm never sure of the order, but I think that the order should be the same as above in render.

[Andras Becsi]

(In reply to comment #12)
> (From update of attachment 178898 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178898&action=review
> 
> > Source/WebKit2/UIProcess/PageViewportController.cpp:264
> > -    drawingArea->setVisibleContentsRect(visibleContentsRect, m_effectiveScale, trajectoryVector);
> > +    drawingArea->setVisibleContentsRect(visibleContentsRect, devicePixelRatio() * m_effectiveScale, trajectoryVector);
> 
> Following our discussion on IRC, I think that this is going to break the viewport meta tag stuff (i.e. if EFL wants to use it with an explicit parameter like we did before this patch).
> Ideally, we would have a way to ask the web process to only scale the backing store resolution and not affect the layout size to avoid having to do this.

Indeed, I can try to come up with something there.

> 
> > Source/WebKit2/UIProcess/qt/QtWebPageSGNode.cpp:87
> >              if (clip->matrix())
> >                  clipMatrix = *clip->matrix();
> > +            clipMatrix.scale(m_window->devicePixelRatio());
> 
> Is it right to scale after the multiplication?
> If the clip matrix has a translation this might not give you the final transform you want.
> I'm never sure of the order, but I think that the order should be the same as above in render.

I had the order the other way round in the previous patch and the clipping had a positioning artifact on the border, switching the order made it correct again.

[Andras Becsi]

Created attachment 179051 [details]
Patch

[Andras Becsi]

Created attachment 179085 [details]
Patch

[Andras Becsi]

Created attachment 179087 [details]
Patch

[Andras Becsi]

(In reply to comment #16)
> Created an attachment (id=179087) [details]
> Patch

Fixed a typo.

[Kenneth Rohde Christiansen]

Comment on attachment 179087 [details]
Patch

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

> Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.cpp:179
> +void CoordinatedLayerTreeHostProxy::setBackingStoreScaleFactor(float scale)

I find the name a bit confusing. You could say that normal scale is also a backing store scale. WebCore uses deviceScaleFactor.

The proper solution would be to actually set deviceScaleFactor in WebCore and modify all our visible content rects to include the factor

[Andras Becsi]

(In reply to comment #18)
> (From update of attachment 179087 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179087&action=review
> 
> > Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.cpp:179
> > +void CoordinatedLayerTreeHostProxy::setBackingStoreScaleFactor(float scale)
> 
> I find the name a bit confusing. You could say that normal scale is also a backing store scale. WebCore uses deviceScaleFactor.
> 
> The proper solution would be to actually set deviceScaleFactor in WebCore and modify all our visible content rects to include the factor

If you look at the patch we need this scale additionally to setting the deviceScaleFactor in WebCore, which we also do in QQuickWebPage::updatePaintNode.
The current device pixel ratio logic is incompatible with the way how the underlying platform abstracts painting to a high definition screen, hence useless for real life hardware.

We should definetely revise it as a whole and try to come up with a soluiton that would allow a custom device pixel ratio and a platform device pixel ratio to coexist, but this kind of a mayor refactor would be too disruptive now and is not feasible for the Qt 5.0 release.

This patch does not affect the behaviour for other ports using this code (currently only EFL) but is a stop-gap solution for QtWebKit on retina display
and currently blocks the release.

I am willing to look into this in more detail after the release.

[Kenneth Rohde Christiansen]

im ok with that, and understands, I just didn't like the name

[Kenneth Rohde Christiansen]

Comment on attachment 179087 [details]
Patch

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

>>> Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.cpp:179
>>> +void CoordinatedLayerTreeHostProxy::setBackingStoreScaleFactor(float scale)
>> 
>> I find the name a bit confusing. You could say that normal scale is also a backing store scale. WebCore uses deviceScaleFactor.
>> 
>> The proper solution would be to actually set deviceScaleFactor in WebCore and modify all our visible content rects to include the factor
> 
> If you look at the patch we need this scale additionally to setting the deviceScaleFactor in WebCore, which we also do in QQuickWebPage::updatePaintNode.
> The current device pixel ratio logic is incompatible with the way how the underlying platform abstracts painting to a high definition screen, hence useless for real life hardware.
> 
> We should definetely revise it as a whole and try to come up with a soluiton that would allow a custom device pixel ratio and a platform device pixel ratio to coexist, but this kind of a mayor refactor would be too disruptive now and is not feasible for the Qt 5.0 release.
> 
> This patch does not affect the behaviour for other ports using this code (currently only EFL) but is a stop-gap solution for QtWebKit on retina display
> and currently blocks the release.
> 
> I am willing to look into this in more detail after the release.

Please add deviceScaleFactor like LayerTreeHostCA

[Kenneth Rohde Christiansen]

Comment on attachment 179087 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:99
> +        d->coordinatedLayerTreeHostProxy()->setBackingStoreScaleFactor(w->devicePixelRatio());

not needed, it can get it directly from the one set by the line below

>>>> Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.cpp:179
>>>> +void CoordinatedLayerTreeHostProxy::setBackingStoreScaleFactor(float scale)
>>> 
>>> I find the name a bit confusing. You could say that normal scale is also a backing store scale. WebCore uses deviceScaleFactor.
>>> 
>>> The proper solution would be to actually set deviceScaleFactor in WebCore and modify all our visible content rects to include the factor
>> 
>> If you look at the patch we need this scale additionally to setting the deviceScaleFactor in WebCore, which we also do in QQuickWebPage::updatePaintNode.
>> The current device pixel ratio logic is incompatible with the way how the underlying platform abstracts painting to a high definition screen, hence useless for real life hardware.
>> 
>> We should definetely revise it as a whole and try to come up with a soluiton that would allow a custom device pixel ratio and a platform device pixel ratio to coexist, but this kind of a mayor refactor would be too disruptive now and is not feasible for the Qt 5.0 release.
>> 
>> This patch does not affect the behaviour for other ports using this code (currently only EFL) but is a stop-gap solution for QtWebKit on retina display
>> and currently blocks the release.
>> 
>> I am willing to look into this in more detail after the release.
> 
> Please add deviceScaleFactor like LayerTreeHostCA

+float CoordinatedLayerTreeHost::deviceScaleFactor() const
+{
+    return m_webPage->corePage()->deviceScaleFactor();
+}

Like this. That is in line with other ports

> Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.cpp:190
> +    const float effectiveBackingStoreScale = m_backingStoreScaleFactor * scale;

scale * deviceScaleFactor()

[Jocelyn Turcotte]

Comment on attachment 179087 [details]
Patch

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

> Source/WebKit2/UIProcess/qt/QtWebPageSGNode.cpp:38
> +    ContentsSGNode(PassRefPtr<LayerTreeRenderer> renderer, const QtWebPageSGNode* parent)

QSGNode already has a parent() method, you could static_cast instead.

> Source/WebKit2/UIProcess/qt/QtWebPageSGNode.cpp:124
> +QtWebPageSGNode::QtWebPageSGNode(const QQuickItem* parent)

The item is not quite the parent of the node, items and nodes are two parallel trees. Calling it "item" would be more appropriate.

[Kenneth Rohde Christiansen]

Comment on attachment 179087 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:95
> +    const QWindow* const w = window();

I think the webkit style is to not use const here, but i dont care much.

Why not use QWindow* window = this->window() ?

> Source/WebKit2/UIProcess/qt/QtWebPageSGNode.cpp:148
> +{
> +    if (m_parent->window())
> +        return m_parent->window()->devicePixelRatio();

why not QWindow* window = m_parent->window() ?

[Kenneth Rohde Christiansen]

Comment on attachment 179087 [details]
Patch

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

> Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.cpp:185
>  void CoordinatedLayerTreeHostProxy::setVisibleContentsRect(const FloatRect& rect, float scale, const FloatPoint& trajectoryVector)

maybe we should rename scale here to pageScaleFactor or so, to make it more clear. I would suggest that

> Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.cpp:195
> +    m_drawingAreaProxy->page()->process()->send(Messages::CoordinatedLayerTreeHost::SetVisibleContentsRect(rect, effectiveBackingStoreScale, trajectoryVector), m_drawingAreaProxy->page()->pageID());

I tink calling it effectiveScaleFactor would be fine!

[Andras Becsi]

Created attachment 179274 [details]
Patch

[Andras Becsi]

Comment on attachment 179274 [details]
Patch

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

> Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.h:106
> +    float m_backingStoreScaleFactor;

D'oh, removing this.

[Kenneth Rohde Christiansen]

Comment on attachment 179274 [details]
Patch

r=me, cq- because of own comment

[Andras Becsi]

Created attachment 179275 [details]
Patch

[Andras Becsi]

Comment on attachment 179275 [details]
Patch

Clearing flags on attachment: 179275

Committed r137597: <http://trac.webkit.org/changeset/137597>

[Andras Becsi]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #30)
> (From update of attachment 179275 [details])
> Clearing flags on attachment: 179275
> 
> Committed r137597: <http://trac.webkit.org/changeset/137597>

Could you remove the related API tests too?

FAIL!  : qmltests::DevicePixelRatio::test_devicePixelRatio() Uncaught exception: Cannot assign to non-existent property "devicePixelRatio"
   Loc: [(0)]
FAIL!  : qmltests::DevicePixelRatio::test_devicePixelRatioMediaQuery() Uncaught exception: Cannot assign to non-existent property "devicePixelRatio"
   Loc: [(0)]

[Andras Becsi]

(In reply to comment #32)
> (In reply to comment #30)
> > (From update of attachment 179275 [details] [details])
> > Clearing flags on attachment: 179275
> > 
> > Committed r137597: <http://trac.webkit.org/changeset/137597>
> 
> Could you remove the related API tests too?
> 
> FAIL!  : qmltests::DevicePixelRatio::test_devicePixelRatio() Uncaught exception: Cannot assign to non-existent property "devicePixelRatio"
>    Loc: [(0)]
> FAIL!  : qmltests::DevicePixelRatio::test_devicePixelRatioMediaQuery() Uncaught exception: Cannot assign to non-existent property "devicePixelRatio"
>    Loc: [(0)]

Ooops, this hunk did apparently not make it into the final patch.
Preparing a patch, sorry for that.

[Andras Becsi]

Test removed in r137605, although webkit-patch got the commit message completely wrong, sigh...