Enable additional middleware scenarios#1147
Conversation
bviglietta
commented
Oct 13, 2016
- Include AudioToolbox.dll in apps using middleware
- Allow UIView animation with spring
- Streamline app protocol activation via Safari view controller
- Allow UIView animation with spring - Streamline app protocol activation via Safari view controller
|
@pradipd is added to the review. #Closed |
| } | ||
|
|
||
| - (void)dealloc { | ||
| _webView.delegate = nil; |
There was a problem hiding this comment.
_webView [](start = 4, length = 8)
Do you have to check if this is null? #ByDesign
There was a problem hiding this comment.
There was a problem hiding this comment.
Since we own our webview, doesn't it not matter that its delegate is being nil'd out? It won't have a word to say about it in just a few more seconds. #ByDesign
There was a problem hiding this comment.
True, but the UIWebView docs specify very sternly that the delegate field must be reset to nil before deallocating the UIWebView, so maybe I should just comply?
In looking at this question, I did discover that UIWebView has a retain cycle involving its callbacks, so that's something that needs to be fixed.
In reply to: 83337136 [](ancestors = 83337136)
There was a problem hiding this comment.
[UIWebView issue addressed in second commit.]
In reply to: 83342864 [](ancestors = 83342864,83337136)
|
|
| // Check whether this request is for the redirect URL that we're watching out for | ||
| // (modulo the query string and fragment, which are allowed to vary) | ||
|
|
||
| if ([url.scheme isEqualToString:_redirectUrl.scheme] && [url.host isEqualToString:_redirectUrl.host] && |
There was a problem hiding this comment.
nit: instead of having a flag variable recombine the ifs #ByDesign
There was a problem hiding this comment.
I could, but I feel the code would be less clear that way.
In reply to: 83669085 [](ancestors = 83669085)
| // straight to the application delegate. | ||
| // Asking the WebView to open such a URL would work, since it would send an activation request | ||
| // to this app, but that would cause an extraneous confirmation dialog from Windows. | ||
| if (appCanOpenURL(url)) { |
There was a problem hiding this comment.
if appCanOpenURL returns FALSE, should OpenWithAppDelegate be set to NO?
If not, do we even need to call appCanOpenURL if OpenWithAppDelegate is already YES?
There was a problem hiding this comment.
First question: no, openWithAppDelegate should be set to YES if either condition is true.
Second question: That's a good point. We can skip the call to appCanOpenURL if openWithAppDelegate is already YES.
In reply to: 83742017 [](ancestors = 83742017)
There was a problem hiding this comment.
Thanks for the clarification.
I'm fine leaving as is or changing it to skip the appCanOpenURL call.
either way is fine.
In reply to: 83745451 [](ancestors = 83745451,83742017)
|
|