Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(iOS): Don't force "UIScrollViewContentInsetAdjustmentNever" (2.x) #530

Conversation

Lindsay-Needs-Sleep
Copy link
Contributor

@Lindsay-Needs-Sleep Lindsay-Needs-Sleep commented Mar 23, 2020

Sister PR for 4.x: PR #531

While spending ridiculous amounts of time on PR #!!!, (and all it's related threads), I also looked into the code that is forcing UIScrollViewContentInsetAdjustmentNever mode.

For most users this behavior would be fine, but after reading @dpogue comments here and through this thread I am convinced that forcing this behavior is incorrect. (Especially, when users can control this themselves via html viewport-fit=cover.)
I have added doc/FAQ.md which describes this in more detail.
The helloWorld Cordova project and the ionic tutorial project uses viewport-fit=cover by default. So this should be pretty standard for cordova users.


There are 2 pieces of code which force UIScrollViewContentInsetAdjustmentNever.

1. Originally committed on 2017-09-19 with the message:

Adjust scroll view behavior for iOS 11

[wkWebView.scrollView setContentInsetAdjustmentBehavior:UIScrollViewContentInsetAdjustmentNever];

2. Originally committed on 2017-09-20 with the message:

Fix for iOS11 initial scroll

Code... Swizzling
@implementation UIScrollView (BugIOS11)

#if __IPHONE_OS_VERSION_MAX_ALLOWED >= 110000

+ (void)load {
    if (@available(iOS 11.0, *)) {
        static dispatch_once_t onceToken;
        dispatch_once(&onceToken, ^{
            Class class = [self class];
            SEL originalSelector = @selector(init);
            SEL swizzledSelector = @selector(xxx_init);

            Method originalMethod = class_getInstanceMethod(class, originalSelector);
            Method swizzledMethod = class_getInstanceMethod(class, swizzledSelector);

            BOOL didAddMethod =
            class_addMethod(class,
                            originalSelector,
                            method_getImplementation(swizzledMethod),
                            method_getTypeEncoding(swizzledMethod));

            if (didAddMethod) {
                class_replaceMethod(class,
                                    swizzledSelector,
                                    method_getImplementation(originalMethod),
                                    method_getTypeEncoding(originalMethod));
            } else {
                method_exchangeImplementations(originalMethod, swizzledMethod);
            }
        });
    }
}

#endif

#pragma mark - Method Swizzling

- (id)xxx_init {
    id a = [self xxx_init];
    if (@available(iOS 11.0, *)) {
        NSArray *stack = [NSThread callStackSymbols];
        for(NSString *trace in stack) {
            if([trace containsString:@"WebKit"]) {
                [a setContentInsetAdjustmentBehavior:UIScrollViewContentInsetAdjustmentNever];
                break;
            }
        }
    }
    return a;
}

@end

Honestly not really sure why both of them exist. But, since they are committed within a day of each other, they were both probably just hurrying to fix the temporary ios 11 bug that prevented viewport-fit=cover from working (CB-12886).

Since it appears the root issue has been solved and released by Apple (at least for iOS 12 and 13 according to my testing), these pieces of code are no longer necessary. (I did not test iOS 11 because I don't have an 11 device, which is because no Apple devices have a max OS of 11, so it should be fairly safe to ignore.)


Environment

I have been testing with:

  • Xcode 10.2

I have been testing on these devices:

  • iPad mini 2, iOS 12.4.5 (real)
  • iPhone 6s, iOS 13.3.1 (real)
  • iPhone X, iOS 12.2 (emulator)

My project is:

  • cordova
  • cordova-plugin-ionic-webview 2.x

I have also tested the 4.x sister PR #531 (which has identical changes basically) with these projects:

modified ionic tutorial

  • cordova-plugin-ionic-webview 4.x
  • cordova-plugin-ionic-keyboard / NO cordova-plugin-ionic-keyboard

modified cordova helloWorld

  • NO cordova-plugin-ionic-keyboard

Summary

Removing these pieces of code:

  • Allow user control of whether ios natively applies the safe inset or not
  • Brings cordova-plugin-ionic-webview closer inline with its progenitor, cordova-plugin-wkwebview-engine

Unrelated: It was way too much work to track down all this information. T.T

This denies users control.
- Remove both methods that force "UIScrollViewContentInsetAdjustmentNever".
- See @dpogue's comments for reasoning: apache/cordova-plugin-wkwebview-engine#108
- Added some documentation that helps users configure their view properly for devices that have safe-insets due to "the notch" like iPhone X.  (So that they can acheive the same behavior as this change was doing if desired).
See PR ionic-team#530 for details
@Lindsay-Needs-Sleep Lindsay-Needs-Sleep force-pushed the fix_2x_dont-force-UIScrollViewContentInsetAdjustmentNever branch from 8a761b6 to a08540d Compare March 23, 2020 11:00
@Lindsay-Needs-Sleep Lindsay-Needs-Sleep changed the title Don't force "UIScrollViewContentInsetAdjustmentNever". feat(iOS): Don't force "UIScrollViewContentInsetAdjustmentNever" (2.x) Mar 23, 2020
@jcesarmobile
Copy link
Member

answer in #531

@jcesarmobile jcesarmobile removed their assignment Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants