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

Please remove org_apache_cordova_UIView_Extension because it isn't necessary since iOS 8. #1399

Closed
hbordersTwitch opened this issue Feb 20, 2024 · 1 comment · Fixed by #1400
Milestone

Comments

@hbordersTwitch
Copy link

Feature Request

Cordova contains org_apache_cordova_UIView_Extension in CDVPlugin.h:

@interface UIView (org_apache_cordova_UIView_Extension)

@property (nonatomic, weak) UIScrollView* scrollView;

@end

with an implementation in CDVPlugin.m:

@implementation UIView (org_apache_cordova_UIView_Extension)

@dynamic scrollView;

- (UIScrollView*)scrollView
{
    SEL scrollViewSelector = NSSelectorFromString(@"scrollView");

    if ([self respondsToSelector:scrollViewSelector]) {
        return ((id (*)(id, SEL))objc_msgSend)(self, scrollViewSelector);
    }

    return nil;
}

@end

It appears Cordova only uses the scrollView property on WKWebView instances, and WKWebView has exposed a scrollView property since iOS8, and Cordova's iOS Platform Guide says iOS 11 is the minimum version required for devices to run Cordova. Thus, this extension and artificial property is unnecessary.

Motivation Behind Feature

Apps integrating Cordova with Swift codebases will not compile if Swift classes feature scrollView members in UIView subclasses with visibility less than public. Integrating Cordova would be easier without this extension.

Alternatives or Workarounds

Integrators can manually delete the @interface from the public header file to avoid compilation issues.

@dpogue dpogue added this to the 8.0.0 milestone Feb 20, 2024
dpogue added a commit to dpogue/cordova-ios that referenced this issue Feb 21, 2024
@dpogue
Copy link
Member

dpogue commented Aug 20, 2024

I'd like to remove this (and have a PR to do so) but unfortunately this is still used by a large number of 3rd party plugins. The problem is that the CDVPlugin class has a webView property that is a UIView*, and we can't type it to a more specific type because the exact type of WebView being created isn't known at compile time (because WebView implementations are provided by plugins).

This category extension was originally added to smooth over the transition from UIWebView to WKWebView, both of which have a scrollView property but which wouldn't be accessed via the more generic UIView*.

In an ideal world, plugins wouldn't need to care about the internal implementation details of the web view or touch its scroll view directly, but sadly most Cordova plugins exist due to the lack of an ideal world. 😞

So removing this entirely is definitely not feasible in the next major version. I can mark it as deprecated, but that still leaves plugins without a good solution (other than copy-pasting this implementation wherever they need it), and doesn't solve the problem of this interfering with Swift view subclasses.

@hbordersTwitch Any suggestions for how we might be able to improve this situation for Swift without breaking compatibility with the existing ecosystem?

dpogue added a commit to dpogue/cordova-ios that referenced this issue Aug 21, 2024
This **removes** the API from Swift to solve the immediate problem in
apacheGH-1399 but leaves it available and deprecated in Objective-C due to use
in 3rd party plugins. (There are only 2 Swift plugins that use this API
as far as I can tell, and neither of them have very high usage or
ongoing maintenance.)
dpogue added a commit to dpogue/cordova-ios that referenced this issue Aug 21, 2024
This **removes** the API from Swift to solve the immediate problem in apache#1399
but leaves it available and deprecated in Objective-C due to use in 3rd party
plugins. (There are only 2 Swift plugins that use this API as far as I can
tell, and neither of them have very high usage or ongoing maintenance.)

Closes apacheGH-1399.
@dpogue dpogue closed this as completed in 39fbf29 Aug 24, 2024
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 a pull request may close this issue.

2 participants