You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by "dpogue (via GitHub)" <gi...@apache.org> on 2024/02/21 19:37:45 UTC
[PR] refactor!: Remove scrollView category extension on UIView [cordova-ios]
dpogue opened a new pull request, #1400:
URL: https://github.com/apache/cordova-ios/pull/1400
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
🚨 **BREAKING API CHANGE!**
This should be tested with all the core plugins, just in case any of them are relying on this API existing.
### Platforms affected
iOS
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
As mentioned in #1399, this API was added as a compatibility shim at the time we supported both UIWebView and WKWebView, when WKWebView did not expose a public `scrollView` property. That property was made available in iOS 8, and this extension now risks causing problems due to being applied globally to all UIView subclasses.
Closes #1399.
### Description
<!-- Describe your changes in detail -->
Remove the category extension to expose a `scrollView` property on all UIView object instances.
### Testing
<!-- Please describe in detail how you tested your changes. -->
...
### Checklist
- [ ] I've run the tests to see all new and existing tests pass
- [ ] I added automated test coverage as appropriate for this change
- [x] 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](https://help.github.com/articles/closing-issues-using-keywords/))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] fix!: Deprecate(/Remove) scrollView category extension on UIView [cordova-ios]
Posted by "dpogue (via GitHub)" <gi...@apache.org>.
dpogue commented on PR #1400:
URL: https://github.com/apache/cordova-ios/pull/1400#issuecomment-2308163766
The equivalent Objective C code would be (copied from the statusbar plugin):
```objc
- (UIScrollView *)webViewScrollView
{
SEL scrollViewSelector = NSSelectorFromString(@"scrollView");
if ([self.webView respondsToSelector:scrollViewSelector]) {
return ((id (*)(id, SEL))objc_msgSend)(self.webView, scrollViewSelector);
}
return nil;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor!: Remove scrollView category extension on UIView [cordova-ios]
Posted by "dpogue (via GitHub)" <gi...@apache.org>.
dpogue commented on PR #1400:
URL: https://github.com/apache/cordova-ios/pull/1400#issuecomment-2288083327
This causes issues for the statusbar plugin which relies on this to access the scroll view without knowing the specific type of the web view implementation.
So putting this on hold for the moment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] fix!: Deprecate(/Remove) scrollView category extension on UIView [cordova-ios]
Posted by "dpogue (via GitHub)" <gi...@apache.org>.
dpogue commented on PR #1400:
URL: https://github.com/apache/cordova-ios/pull/1400#issuecomment-2300296922
The best short term solution seems to be the following:
* Remove the extension from being visible to Swift code. This solves the problem raised in #1399
* This breaks any Swift plugins that use this API, but there seem to only be a few and they are not maintained and don't have high download counts
* Mark the Objective C method as deprecated so that existing 3rd party plugins that call it will see warnings
* Prepare to remove this entirely in Cordova iOS 9 as per the deprecation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] fix!: Deprecate(/Remove) scrollView category extension on UIView [cordova-ios]
Posted by "dpogue (via GitHub)" <gi...@apache.org>.
dpogue merged PR #1400:
URL: https://github.com/apache/cordova-ios/pull/1400
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor!: Remove scrollView category extension on UIView [cordova-ios]
Posted by "jcesarmobile (via GitHub)" <gi...@apache.org>.
jcesarmobile commented on PR #1400:
URL: https://github.com/apache/cordova-ios/pull/1400#issuecomment-2299812688
I don’t see what’s the problem with introducing a breaking change.
Changing the template to swift is also breaking to some plugins and it’s merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor!: Remove scrollView category extension on UIView [cordova-ios]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1400:
URL: https://github.com/apache/cordova-ios/pull/1400#issuecomment-1957779484
## [Codecov](https://app.codecov.io/gh/apache/cordova-ios/pull/1400?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`92017bb`)](https://app.codecov.io/gh/apache/cordova-ios/commit/92017bbde3a4b6d4c62187f6804743224b7fe9c9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 78.25% compared to head [(`f7d069d`)](https://app.codecov.io/gh/apache/cordova-ios/pull/1400?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 78.25%.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #1400 +/- ##
=======================================
Coverage 78.25% 78.25%
=======================================
Files 15 15
Lines 1789 1789
=======================================
Hits 1400 1400
Misses 389 389
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cordova-ios/pull/1400?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor!: Remove scrollView category extension on UIView [cordova-ios]
Posted by "dpogue (via GitHub)" <gi...@apache.org>.
dpogue commented on PR #1400:
URL: https://github.com/apache/cordova-ios/pull/1400#issuecomment-2298210243
Status Bar plugin is fixed, but this API is used in a number of 3rd party plugins as well... I feel like our only option is going to be deprecation instead of removal (which doesn't actually address the issue that was raised about this) 😞
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org