You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by agrieve <gi...@git.apache.org> on 2014/10/29 15:32:56 UTC

[GitHub] cordova-ios pull request: Unplug whitelist - review

GitHub user agrieve opened a pull request:

    https://github.com/apache/cordova-ios/pull/116

    Unplug whitelist - review

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/apache/cordova-ios unplug-whitelist

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-ios/pull/116.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #116
    
----
commit 0c013f6915580b69d35332c16fbd7079675a1e93
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-23T20:02:59Z

    Seperate config.xml parsing logic
    
    Separates the logic of finding config.xml and feeding its contents to a parser from the task of choosing that parser and using its results

commit 83ba4cd06d6ba6b6db125b6dabe00176fda8d9b3
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-24T20:09:07Z

    Add hooks for whitelist plugins
    
    This allows plugins to implement two methods:
    - (BOOL)shouldAllowRequestForURL:(NSURL *)url
    - (BOOL)shouldAllowNavigationToURL:(NSURL *)url
    - (BOOL)shouldOpenExternalURL:(NSURL *)url

commit 5b41cd6fa45567551905ad43547535e52158132a
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-24T20:13:47Z

    Remove whitelist objects from CDVURLProtocol and CDVViewController classes

commit cc7c53deb4beb5cff57c6385f5d50f8bf748283f
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-24T20:14:15Z

    Remove whitelist config.xml parsing

commit 13d90874087116f07bbeacdbb4629228d9cf8153
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-27T14:59:40Z

    Defer whitelist decisions to plugins

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-ios pull request: Unplug whitelist - review

Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on a diff in the pull request:

    https://github.com/apache/cordova-ios/pull/116#discussion_r19574303
  
    --- Diff: CordovaLib/Classes/CDVViewController.m ---
    @@ -714,24 +712,44 @@ - (BOOL)webView:(UIWebView*)theWebView shouldStartLoadWithRequest:(NSURLRequest*
         }
     
         /*
    -     * If a URL is being loaded that's a file/http/https URL, just load it internally
    +     *    If we loaded the HTML from a string, we let the app handle it
          */
    -    if ([url isFileURL]) {
    +    if (self.loadFromString == YES) {
    --- End diff --
    
    || : self.loadFromString != NO 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-ios pull request: Unplug whitelist - review

Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on the pull request:

    https://github.com/apache/cordova-ios/pull/116#issuecomment-61010261
  
    Also important to note that the tests are now failing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-ios pull request: Unplug whitelist - review

Posted by clelland <gi...@git.apache.org>.
Github user clelland commented on a diff in the pull request:

    https://github.com/apache/cordova-ios/pull/116#discussion_r19623071
  
    --- Diff: CordovaLib/Classes/CDVViewController.m ---
    @@ -714,24 +712,44 @@ - (BOOL)webView:(UIWebView*)theWebView shouldStartLoadWithRequest:(NSURLRequest*
         }
     
         /*
    -     * If a URL is being loaded that's a file/http/https URL, just load it internally
    +     *    If we loaded the HTML from a string, we let the app handle it
          */
    -    if ([url isFileURL]) {
    +    if (self.loadFromString == YES) {
    +        self.loadFromString = NO;
             return YES;
         }
     
         /*
    -     *    If we loaded the HTML from a string, we let the app handle it
    +     * Handle all other types of urls (tel:, sms:), and requests to load a url in the main webview.
          */
    -    else if (self.loadFromString == YES) {
    --- End diff --
    
    Also, this was the original source of the "== YES"; git diff just makes it *look* like I added that ;)
    
    I'll pull the other instances out of the file while I'm here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-ios pull request: Unplug whitelist - review

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-ios/pull/116#discussion_r19541663
  
    --- Diff: CordovaLib/Classes/CDVViewController.m ---
    @@ -749,24 +767,77 @@ - (BOOL)webView:(UIWebView*)theWebView shouldStartLoadWithRequest:(NSURLRequest*
             return YES;
         }
     
    -    /*
    -     * Handle all other types of urls (tel:, sms:), and requests to load a url in the main webview.
    -     */
    -    else {
    -        if ([self.whitelist schemeIsAllowed:[url scheme]]) {
    -            return [self.whitelist URLIsAllowed:url];
    -        } else {
    -            if ([[UIApplication sharedApplication] canOpenURL:url]) {
    -                [[UIApplication sharedApplication] openURL:url];
    -            } else { // handle any custom schemes to plugins
    -                [[NSNotificationCenter defaultCenter] postNotification:[NSNotification notificationWithName:CDVPluginHandleOpenURLNotification object:url]];
    +    return NO;
    +}
    +
    +- (BOOL)shouldAllowRequestForURL:(NSURL *)url
    +{
    +    BOOL anyPluginsResponded = NO;
    +    BOOL shouldAllowRequest = NO;
    +    for (NSString* pluginName in pluginObjects) {
    +        CDVPlugin* plugin = [pluginObjects objectForKey:pluginName];
    +        SEL selector = NSSelectorFromString(@"shouldAllowRequestForURL:");
    --- End diff --
    
    You should document these new selectors in CDVPlugin.h. When doing so, make note that shouldAllowRequestForURL doesn't work with websocket, or at all for WKWebView


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-ios pull request: Unplug whitelist - review

Posted by clelland <gi...@git.apache.org>.
Github user clelland commented on a diff in the pull request:

    https://github.com/apache/cordova-ios/pull/116#discussion_r19543890
  
    --- Diff: CordovaLib/Classes/CDVViewController.m ---
    @@ -714,24 +712,44 @@ - (BOOL)webView:(UIWebView*)theWebView shouldStartLoadWithRequest:(NSURLRequest*
         }
     
         /*
    -     * If a URL is being loaded that's a file/http/https URL, just load it internally
    +     *    If we loaded the HTML from a string, we let the app handle it
          */
    -    if ([url isFileURL]) {
    +    if (self.loadFromString == YES) {
    --- End diff --
    
    Yes. Yes it can :) Thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-ios pull request: Unplug whitelist - review

Posted by clelland <gi...@git.apache.org>.
Github user clelland commented on the pull request:

    https://github.com/apache/cordova-ios/pull/116#issuecomment-61135482
  
    That's a terrible error, btw :)
    
        EX_DATAERR (65)       The input data was incorrect in some way.  This should only be used for user's data and not system files.
    
    I'll look into it and see if it's something related to this branch. Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-ios pull request: Unplug whitelist - review

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-ios/pull/116#discussion_r19541386
  
    --- Diff: CordovaLib/Classes/CDVViewController.m ---
    @@ -714,24 +712,44 @@ - (BOOL)webView:(UIWebView*)theWebView shouldStartLoadWithRequest:(NSURLRequest*
         }
     
         /*
    -     * If a URL is being loaded that's a file/http/https URL, just load it internally
    +     *    If we loaded the HTML from a string, we let the app handle it
          */
    -    if ([url isFileURL]) {
    +    if (self.loadFromString == YES) {
    --- End diff --
    
    nit: use if (self.loadFromString) rather than == YES. ==YES can actually bite if BOOL has a value of 2 .


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-ios pull request: Unplug whitelist - review

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve closed the pull request at:

    https://github.com/apache/cordova-ios/pull/116


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org