Bug 74417 – [Qt] DeviceOrientationClientMockQt should be removed in favor of DeviceOrientationClientMock

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Alexander Færøy]

SSIA.

Patch coming up.

[Alexander Færøy]

Created attachment 119089 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 119089 [details]
Patch

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

> Source/WebCore/ChangeLog:6
> +        [Qt] DeviceOrientationClientMockQt should be removed in favor of DeviceOrientationClientMock
> +        https://bugs.webkit.org/show_bug.cgi?id=74417
> +
> +        Reviewed by NOBODY (OOPS!).

As this is based on my code and I did it together with you :-) You should add me and I shouldnt review it officially.

> Source/WebKit/qt/Api/qwebframe.cpp:474
>      WebCore::Frame* frame = core(q);
>  
>      switch (m_orientation.reading()->orientation()) {
> -    case QtMobility::QOrientationReading::TopUp:
> +    case QOrientationReading::TopUp:
>          orientation = 0;
>          break;
> -    case QtMobility::QOrientationReading::TopDown:
> +    case QOrientationReading::TopDown:
>          orientation = 180;
>          break;
> -    case QtMobility::QOrientationReading::LeftUp:
> +    case QOrientationReading::LeftUp:
>          orientation = -90;
>          break;
> -    case QtMobility::QOrientationReading::RightUp:
> +    case QOrientationReading::RightUp:
>          orientation = 90;
>          break;
> -    case QtMobility::QOrientationReading::FaceUp:
> -    case QtMobility::QOrientationReading::FaceDown:
> +    case QOrientationReading::FaceUp:
> +    case QOrientationReading::FaceDown:
>          // WebCore unable to handle it
>      default:
>          return;

All this code is wrong. This is for the browser window orientation on the device. It may not always follow device orientation as you could for instance have a finger on the window and block it from changing browser window orientation

> Source/WebKit/qt/Api/qwebframe_p.h:40
> +#if ENABLE(ORIENTATION_EVENTS)

ORIENTATION_EVENTS are for browser orientation! But if this is not your code (ie unrelated to the patch) it can be fixed in a follow up

> Source/WebKit/qt/Api/qwebpage.cpp:326
> +    if (QWebPagePrivate::drtRun)
> +        pageClients.deviceOrientationClient = new DeviceOrientationClientMock;
> +    else
> +        pageClients.deviceOrientationClient = new DeviceOrientationClientQt;

Maybe the below is clearer?

bool useMock = QWebPagePrivate::drtRun;

pageClients.deviceOrientationClient = useMock ? new DeviceOrientationClientMock : new DeviceOrientationClientQt;

?

[Alexander Færøy]

Created attachment 119177 [details]
Patch

[Simon Hausmann]

Comment on attachment 119177 [details]
Patch

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

r=me but watch the bot when landing :)

> Tools/DumpRenderTree/qt/LayoutTestControllerQt.cpp:853
> +    QList<WebCore::WebPage*> pages = m_drt->getAllPages();
> +    foreach (WebCore::WebPage* page, pages)
> +        DumpRenderTreeSupportQt::setMockDeviceOrientation(page, canProvideAlpha, alpha, canProvideBeta, beta, canProvideGamma, gamma);

The loop could also be moved into setMockDeviceOrientation.

[WebKit Review Bot]

Comment on attachment 119177 [details]
Patch

Clearing flags on attachment: 119177

Committed r102755: <http://trac.webkit.org/changeset/102755>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.