feat(ios): add support for ios 16 #107

oofaish · 2022-10-01T15:34:42Z

Platforms affected

Apple devices running iOS 16

Motivation and Context

resolves #100

Currently orientation is broken on iOS 16 with
Setting UIDevice.orientation is not supported. Please use UIWindowScene.requestGeometryUpdate(_:)

Description

deprecate on whether iOS 16 is available - either use the old method or the new one.

Testing

confirmed on iPhone 11 running iOS 15 and iPhone 13 Pro running iOS 16.
I have not confirmed all features work, but simply setting orientations works - the old pre iOS 16 should be unaffected.

Checklist

I've run the tests to see all new and existing tests pass
I added automated test coverage as appropriate for this change
Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
I've updated the documentation if necessary

src/ios/CDVOrientation.m

Theblood · 2022-10-31T00:29:21Z

Please Confirm this PR!!

jindrichkuba · 2022-11-02T12:51:40Z

can u please release this change?

webfletch · 2022-11-03T22:41:55Z

What is holding up the release of this change?

fernandaj222 · 2022-11-08T17:08:49Z

Hey, do you have any news about the merge of this PR? or something about the fix to screen orientation on IOS?

thmclellan · 2022-11-08T20:42:39Z

Thanks @oofaish for sharing your fix! I've been testing on iOS 16.1 on a physical iPhone 13 Pro Max (Capacitor 4.4, Ionic 5, Angular 12, XCode 14.1, Ventura 13.0) and noticed two issues:

1 - Starting from primary portrait without my screen locked, calling window.screen.orientation.lock("landscape") causes a strange double rotation (see attached video). Calling window.screen.orientation.lock("landscape-primary") works as expected.

From debugging, it seems L68 (UIInterfaceOrientationMaskLandscapeLeft) is getting called in the general landscape scenario, while L74 UIInterfaceOrientationMaskLandscapeRight is getting called on landscape-primary.

2 - Calling window.screen.orientation.unlock() is causing a requestGeometryUpdate error in Xcode:

2022-11-08 11:44:59.124078-0800 App[3407:1282732] [Orientation] BUG IN CLIENT OF UIKIT: Setting UIDevice.orientation is not supported. Please use UIWindowScene.requestGeometryUpdate(_:)

It seems L93 is being called on iOS 16:
[[UIDevice currentDevice] setValue:[NSNumber numberWithInt:_lastOrientation] forKey:@"orientation"];

XCode complains but still executes L96 (setNeedsUpdateOfSupportedInterfaceOrientations) and presumably that does the unlock.

This is minor stuff but wanted to share what I found while testing. Thanks again

landscape-double-rotation.mov

oofaish · 2022-11-09T11:32:03Z

thanks @thmclellan I can have a look in the next couple of days

oofaish · 2022-11-11T21:48:18Z

@thmclellan the latest commit in the branch fixes the double rotation issue - the code is definitely a bit messy now - but seems to be working fine on the cases I have tried.

erisu

Overall it might be OK, except for the files that shouldnt be changed in the PR.

But as you suggested in a previous commit,

the code is definitely a bit messy now - but seems to be working fine

What if you split the code out into seperate methods?

-(void)handleAboveEqualIos16
{
    NSObject *value;

    if (orientationMask != 15) {
        ...
    } else {
        ...
    }

    if (value != nil) {
        _isLocked = true;
        UIWindowScene *scene = (UIWindowScene*)[[UIApplication.sharedApplication connectedScenes] anyObject];
        ...
    } else {
        _isLocked = false;
    }
}

-(void)handleBelowEqualIos15
{
    NSValue *value;

    if (orientationMask != 15) {
        ...
    } else {
        ...
    }

    if (value != nil) {
        _isLocked = true;
        [[UIDevice currentDevice] setValue:value forKey:@"orientation"];
    } else {
        _isLocked = false;
    }
}

And then in the screenOrientation write

if ([UIDevice currentDevice] != nil){
    if (@available(iOS 16.0, *)) {
        [self handleAboveEqualIos16];
    } else {
        [self handleBelowEqualIos15];
    }
}

It makes some of the code look repeative but it keeps most of the iOS 15 and below and 16 and above code seperated. Not all is split out but the main chunk of it.

Also you no longer need to have this value and value16 name since its all split as well.. keep it simple as value

package.json

package-lock.json

thmclellan · 2022-11-14T00:42:58Z

Thanks @oofaish for these changes. Tested on iOS 16 physical iPhone 13 Pro Max and the "landscape" lock is working as expected now. The "unlock" call still results in the below xcode warning but it's working fine from a user standpoint:

[Orientation] BUG IN CLIENT OF UIKIT: Setting UIDevice.orientation is not supported. Please use UIWindowScene.requestGeometryUpdate(_:))

undimon · 2022-11-21T12:52:31Z

[scene requestGeometryUpdateWithPreferences:(UIWindowSceneGeometryPreferencesIOS*)value16 errorHandler:^(NSError * _Nonnull error) { NSLog(@"Failed to change orientation %@ %@", error, [error userInfo]); // do nothing }];

I get this error:
None of the requested orientations are supported by the view controller

marco-hd · 2022-11-22T10:24:16Z

iPhone XR / iOS 16.1.1 / XCode 14.1

Hi guys,

I can confirm:

rotation is working
double rotation is fixed

However, the unlocking doesn't happen anymore.

I have no experience with Obj-C (please bear with me), but I tried to make it work this way:

...
if (_lastOrientation != UIInterfaceOrientationUnknown) {                                        
    ((void (*)(CDVViewController*, SEL, NSMutableArray*))objc_msgSend)(vc,selector,result);
    if (@available(iOS 16.0, *)) {
        [self.viewController setNeedsUpdateOfSupportedInterfaceOrientations];
        value16 = [[UIWindowSceneGeometryPreferencesIOS alloc] initWithInterfaceOrientations:_lastOrientation];
    }
    else {
        [[UIDevice currentDevice] setValue:[NSNumber numberWithInt:_lastOrientation] forKey:@"orientation"];                      
        [UINavigationController attemptRotationToDeviceOrientation];
    }
}
...

and the _lastOrientation initialisation:

...
if (!_isLocked) {
      if (@available(iOS 16.0, *)) {
          UIWindowScene *scene = (UIWindowScene*)[[UIApplication.sharedApplication connectedScenes] anyObject];

          switch(scene.interfaceOrientation) {
              case UIInterfaceOrientationUnknown:
                  _lastOrientation = UIInterfaceOrientationMaskAll;
                  break;
              case UIInterfaceOrientationPortrait:
                  _lastOrientation = UIInterfaceOrientationMaskPortrait;
                  break;
              case UIInterfaceOrientationPortraitUpsideDown:
                  _lastOrientation = UIInterfaceOrientationMaskPortraitUpsideDown;
                  break;
              case UIInterfaceOrientationLandscapeLeft:
                  _lastOrientation = UIInterfaceOrientationMaskLandscapeLeft;
                  break;
              case UIInterfaceOrientationLandscapeRight:
                  _lastOrientation = UIInterfaceOrientationMaskLandscapeRight;
                  break;
          }

      } else {
          _lastOrientation = [UIApplication sharedApplication].statusBarOrientation;        
      }      
}
...

So, it kind of works, but the locked screen can be unlocked by pure rotation of the device.
Any idea how to keep it locked until it is explicitly unlocked from the JS WebView world?

VasanthCBH · 2022-12-07T07:32:53Z

Any update about the merge of this PR or a fix for this screen orientation issue on IOS?

oofaish · 2022-12-07T10:28:31Z

Ooh thanks for the review @erisu. I can take a look. the changes that you say should not be there were due to another commit that was not in master it seems. Weird. I ll sort it out.

Theblood

Please Change

…y always calling setSupportedOrientations but only call setNeedsUpdateOfSupportedInterfaceOrientations at the end to avoid refreshing orientation twice

oofaish · 2022-12-10T11:07:40Z

@erisu here is an updated PR:

go back and fix the issue with locking not working (had to call ((void (*)(CDVViewController*, SEL, NSMutableArray*))objc_msgSend)(vc,selector,result); even in iOS16
only call setNeedsUpdateOfSupportedInterfaceOrientations at the end so we dont end up double rotating - seems to work fine
split the code into post ios16 and pre ios16 as requested.

confirmed locking/unlocking works on iOS15 and 16.

src/ios/CDVOrientation.m

oofaish · 2022-12-10T16:44:10Z

Have a look at latest PR see what you think @erisu - appreciate all the comments.

oofaish · 2022-12-11T08:46:10Z

Hmm, looks like the tests failed again? 😡

oofaish · 2022-12-11T08:53:08Z

should I be using

#if __IPHONE_OS_VERSION_MAX_ALLOWED >= 160000

instead of

#if __IPHONE_OS_VERSION_MAX_ALLOWED > __IPHONE_15_5

confirmed if I use >= 170000 on xCode vs >= 160000 it will switch branches in xCode14, but not with __IPHONE_15_5 vs __IPHONE_16_5, not the perfect test, but there it is

erisu · 2022-12-11T09:28:05Z

should I be using
#if __IPHONE_OS_VERSION_MAX_ALLOWED >= 160000
instead of
#if __IPHONE_OS_VERSION_MAX_ALLOWED > __IPHONE_15_5
confirmed if I use >= 170000 on xCode vs >= 160000 it will switch branches in xCode14, but not with __IPHONE_15_5 vs __IPHONE_16_5, not the perfect test, but there it is

Awesome, the update fixes the CI and everything is now green.

The changes look great, clean, and readable to me.

The only other small nitpick that I had was the variable name value16 could be renamed to value. Since the code was split out per iOS version range, there are no more overlapping variable name usages.

But, other than that, everything is great.

oofaish · 2022-12-11T09:47:39Z

thanks @erisu - changed the variable name too

oofaish · 2022-12-11T13:19:34Z

Can someone with write access merge this PR? 🤗

erisu · 2022-12-11T13:44:07Z

Can someone with write access merge this PR? 🤗

I will merge it in by tomorrow (JST).
I am waiting for about 12hrs (since my last approval) to give other PMC members a chance to leave a review.

oofaish · 2022-12-11T14:41:29Z

🙏

breautek

I'm not in a position to be able to physically test these changes as I don't have access to a mac, so I'll be relying on community members to make that distinction...

But codewise everything appears okay.

matthew2564 · 2022-12-13T08:58:04Z

Will this fix be getting published via NPM? The latest version there is 3.0.2 - cordova-plugin-screen-orientation.

oofaish · 2022-12-14T10:48:39Z

@erisu do I need to create a new tag? can we do this manually or should I be putting up another PR?

breautek · 2022-12-14T14:49:23Z

Will this fix be getting published via NPM? The latest version there is 3.0.2 - cordova-plugin-screen-orientation.

It will be, but we cannot provide an ETA.

do I need to create a new tag? can we do this manually or should I be putting up another PR?

Nope, the tags are all managed by the release processed.

shruti-talekar · 2022-12-15T10:51:32Z

@oofaish - I tested this solution on iPhone 13 Pro Max v16.1.1. But the fix is not working. I am getting black(BLANK) screen when device is rotated to Landscape mode. What could be the issue? Can you please help

oofaish · 2022-12-15T11:11:46Z

@shruti-talekar please open a new issue and describe the problem, how you compiled the code, any error messages, etc - my tests have been working fine I am afraid.

guillaumeperaldi reviewed Oct 4, 2022

View reviewed changes

src/ios/CDVOrientation.m Show resolved Hide resolved

ghost approved these changes Oct 11, 2022

View reviewed changes

fernandaj222 mentioned this pull request Oct 28, 2022

ScreenOrientation Cordova Plugin not working anymore on IOS 16 (Simulator/Real) #108

Closed

3 tasks

webfletch approved these changes Oct 31, 2022

View reviewed changes

Theblood approved these changes Nov 2, 2022

View reviewed changes

erisu changed the title ~~(iOS) fix(ios): add support for ios 16~~ feat(ios): add support for ios 16 Nov 13, 2022

erisu requested changes Nov 13, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

package-lock.json Outdated Show resolved Hide resolved

package-lock.json Outdated Show resolved Hide resolved

Theblood suggested changes Dec 9, 2022

View reviewed changes

oofaish force-pushed the ios-16 branch from 931974e to b5f5e67 Compare December 10, 2022 10:41

break up ios16 into a seperate function + fix the unlock/lock issue b…

2d08636

…y always calling setSupportedOrientations but only call setNeedsUpdateOfSupportedInterfaceOrientations at the end to avoid refreshing orientation twice

oofaish force-pushed the ios-16 branch from b5f5e67 to 2d08636 Compare December 10, 2022 10:44

oofaish requested a review from erisu December 10, 2022 11:08

erisu reviewed Dec 10, 2022

View reviewed changes

src/ios/CDVOrientation.m Outdated Show resolved Hide resolved

let us compile in xcode < 14

1f09253

oofaish requested a review from erisu December 10, 2022 16:44

rewrite the iOS15 vs 16 check

222db8d

erisu approved these changes Dec 11, 2022

View reviewed changes

reword variable name

dd59d2b

breautek approved these changes Dec 11, 2022

View reviewed changes

erisu merged commit b444df6 into apache:master Dec 12, 2022

This comment was marked as off-topic.

Sign in to view

apache locked as resolved and limited conversation to collaborators Dec 16, 2022

breautek added this to the 3.0.3 milestone Dec 16, 2022

feat(ios): add support for ios 16 #107

feat(ios): add support for ios 16 #107

Conversation

oofaish commented Oct 1, 2022 • edited Loading

Platforms affected

Motivation and Context

Description

Testing

Checklist

Theblood commented Oct 31, 2022

jindrichkuba commented Nov 2, 2022

webfletch commented Nov 3, 2022

fernandaj222 commented Nov 8, 2022

thmclellan commented Nov 8, 2022

oofaish commented Nov 9, 2022

oofaish commented Nov 11, 2022

erisu left a comment • edited Loading

Choose a reason for hiding this comment

thmclellan commented Nov 14, 2022

undimon commented Nov 21, 2022

marco-hd commented Nov 22, 2022

VasanthCBH commented Dec 7, 2022

oofaish commented Dec 7, 2022

Theblood left a comment

Choose a reason for hiding this comment

oofaish commented Dec 10, 2022 • edited Loading

oofaish commented Dec 10, 2022

oofaish commented Dec 11, 2022

oofaish commented Dec 11, 2022 • edited Loading

erisu commented Dec 11, 2022 • edited Loading

oofaish commented Dec 11, 2022

oofaish commented Dec 11, 2022

erisu commented Dec 11, 2022 • edited Loading

oofaish commented Dec 11, 2022

breautek left a comment

Choose a reason for hiding this comment

matthew2564 commented Dec 13, 2022 • edited Loading

oofaish commented Dec 14, 2022

breautek commented Dec 14, 2022

shruti-talekar commented Dec 15, 2022 • edited Loading

oofaish commented Dec 15, 2022

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

oofaish commented Oct 1, 2022 •

edited

Loading

erisu left a comment •

edited

Loading

oofaish commented Dec 10, 2022 •

edited

Loading

oofaish commented Dec 11, 2022 •

edited

Loading

erisu commented Dec 11, 2022 •

edited

Loading

erisu commented Dec 11, 2022 •

edited

Loading

matthew2564 commented Dec 13, 2022 •

edited

Loading

shruti-talekar commented Dec 15, 2022 •

edited

Loading