Bug 133530 – [iOS][WK2] Add device orientation

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[iOS][WK2] Add device orientation

[Benjamin Poulain]

Created attachment 232517 [details]
Patch

[Benjamin Poulain]

<rdar://problem/16680041>

[WebKit Commit Bot]

Attachment 232517 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebView/WebFrame.mm:1273:  The parameter name "frame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Benjamin Poulain]

Created attachment 232518 [details]
Patch

[WebKit Commit Bot]

Attachment 232518 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebView/WebFrame.mm:1273:  The parameter name "frame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Comment on attachment 232518 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:193
> +        return 0;

sad that we don't have a comparable enum and use integer degrees instead, but not your fault

> Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:287
> +    if (deviceOrientation != m_deviceOrientation) {

This (and possibly those above) needs to check isValid, I think? (what happens if the WebProcess is dead when we get a UIWindowDidRotateNotification?)

[Benjamin Poulain]

Committed r169625: <http://trac.webkit.org/changeset/169625>

[Darin Adler]

Comment on attachment 232518 [details]
Patch

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

> Source/WebCore/page/Frame.cpp:292
> +    for (Frame* frame = this; frame; frame = frame->tree().traverseNext())
> +        frames.append(*frame);

This is peculiar. It iterates starting with this frame, but continues up to this frames siblings and parents if the frame is not the main frame. I think this function is meant to be called only on the main frame, so if we do want it on the frame, then it should be on the MainFrame class.

But actually we try to avoid putting these directly into Frame these days, so normally we’d put this into something like EventHandler.

> Source/WebCore/page/Frame.cpp:297
> +    for (unsigned i = 0; i < frames.size(); i++) {
> +        if (Document* doc = frames[i]->document())
> +            doc->dispatchWindowEvent(Event::create(eventNames().orientationchangeEvent, false, false));
> +    }

This is old fashioned. Here’s the modern way to write it:

    for (auto& frame : frames) {
        if (Document* document = frame->document())
            document->dispatchWindowEvent(Event::create(eventNames().orientationchangeEvent, false, false));
    }