You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Giorgioshooter <gi...@git.apache.org> on 2017/06/20 19:20:42 UTC

[GitHub] cordova-ios pull request #321: CB-12937 - (iOS) added new method handleOpenU...

GitHub user Giorgioshooter opened a pull request:

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

    CB-12937 - (iOS) added new method handleOpenURLWithApplicationSourceAndAnno…

    …tation in CDVPlugin and new notification id CDVPluginHandleOpenURLWithAppSourceAndAnnotationNotification which is broadcasted from CDVAppDelegate openURL method.
    
    <!--
    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!
    -->
    
    ### Platforms affected
    iOS
    
    ### What does this PR do?
    adds a new public method handleOpenURLWithApplicationSourceAndAnnotation in CDVPlugin which contains the URL the applicationSource and annotation 
    
    ### What testing has been done on this change?
    Run test suite for cordova-ios 
    tested on an example app to make sure that the original  handleOpenURL. 
    I did not find any tests on CDVPlugin 
    
    ### Checklist
    - [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
    - [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
    - [ ] Added automated test coverage as appropriate for this change.

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

    $ git pull https://github.com/Giorgioshooter/cordova-ios CB-12937-handleOpenURL-with-applicationSource-and-annotation

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

    https://github.com/apache/cordova-ios/pull/321.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 #321
    
----
commit f49771bfa084b1b64965ee97a3b4afbf407693c3
Author: Georgios Galatoulas <ge...@gmail.com>
Date:   2017-06-20T12:35:40Z

    CB-12937 - added new method handleOpenURLWithApplicationSourceAndAnnotation in CDVPlugin and new notification id CDVPluginHandleOpenURLWithAppSourceAndAnnotationNotification which is broadcasted from CDVAppDelegate openURL method.

----


---
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 #321: CB-12937 - (iOS) added new method handleOpenU...

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

    https://github.com/apache/cordova-ios/pull/321#discussion_r123074679
  
    --- Diff: CordovaLib/Classes/Public/CDVAppDelegate.m ---
    @@ -78,9 +78,23 @@ - (BOOL)application:(UIApplication*)application openURL:(NSURL*)url sourceApplic
         if (!url) {
             return NO;
         }
    +    
    +    NSMutableDictionary * openURLData = [[NSMutableDictionary alloc] init];
    +    
    +    [openURLData setValue:url forKey:@"url"];
    +    
    +    if (sourceApplication) {
    +        [openURLData setValue:sourceApplication forKey:@"sourceApplication"];
    +    }
    +    
    +    if (annotation) {
    +        [openURLData setValue:annotation forKey:@"annotation"];
    +    }
     
    --- End diff --
    
    Also with the literal, you can just make it a NSDictionary 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 #321: CB-12937 - (iOS) added new method handleOpenU...

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

    https://github.com/apache/cordova-ios/pull/321#discussion_r123078350
  
    --- Diff: CordovaLib/Classes/Public/CDVAppDelegate.m ---
    @@ -78,9 +78,23 @@ - (BOOL)application:(UIApplication*)application openURL:(NSURL*)url sourceApplic
         if (!url) {
             return NO;
         }
    +    
    +    NSMutableDictionary * openURLData = [[NSMutableDictionary alloc] init];
    +    
    +    [openURLData setValue:url forKey:@"url"];
    +    
    +    if (sourceApplication) {
    +        [openURLData setValue:sourceApplication forKey:@"sourceApplication"];
    +    }
    +    
    +    if (annotation) {
    +        [openURLData setValue:annotation forKey:@"annotation"];
    +    }
     
    --- End diff --
    
    Yeah I was between this or adding the fields if they exist. I will change it to NSDictionary with literal.  


---
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 #321: CB-12937 - (iOS) added new method handleOpenU...

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

    https://github.com/apache/cordova-ios/pull/321#discussion_r123073237
  
    --- Diff: CordovaLib/Classes/Public/CDVAppDelegate.m ---
    @@ -78,9 +78,23 @@ - (BOOL)application:(UIApplication*)application openURL:(NSURL*)url sourceApplic
         if (!url) {
             return NO;
         }
    +    
    +    NSMutableDictionary * openURLData = [[NSMutableDictionary alloc] init];
    +    
    +    [openURLData setValue:url forKey:@"url"];
    +    
    +    if (sourceApplication) {
    +        [openURLData setValue:sourceApplication forKey:@"sourceApplication"];
    +    }
    +    
    +    if (annotation) {
    +        [openURLData setValue:annotation forKey:@"annotation"];
    +    }
     
    --- End diff --
    
    Personally I would have used the dictionary literals here to be more clear:
    ```
    NSMutableDictionary* openURLData = @{
         @"url" : url,
         @"sourceApplication": sourceApplication,
         @"annotation": annotation
    };
    ```


---
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 #321: CB-12937 - (iOS) added new method handleOpenU...

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

    https://github.com/apache/cordova-ios/pull/321#discussion_r123078485
  
    --- Diff: CordovaLib/Classes/Public/CDVPlugin.m ---
    @@ -140,6 +142,36 @@ - (void)handleOpenURL:(NSNotification*)notification
         }
     }
     
    +/*
    +    NOTE: calls into JavaScript must not call or trigger any blocking UI, like alerts
    + */
    +- (void)handleOpenURLWithApplicationSourceAndAnnotation: (NSNotification*)notification
    +{
    +    
    +    // override to handle urls sent to your app
    +    // register your url schemes in your App-Info.plist
    +    
    +    // The notification object is an NSDictionary which contains
    +    // - url which is a type of NSURL
    +    // - sourceApplication which is a type of NSString and represents the package
    +    // id of the app that calls our app
    +    // - annotation which a type of Property list which can be several different types
    +    // please see https://developer.apple.com/library/content/documentation/General/Conceptual/DevPedia-CocoaCore/PropertyList.html
    +    
    +    NSDictionary*  notificationData = [notification object];
    +    
    +    if ([notificationData isKindOfClass: NSDictionary.class]){
    +        
    +        NSURL* url = notificationData[@"url"];
    +        NSString* sourceApplication = notificationData[@"sourceApplication"];
    --- End diff --
    
    No problem at all.


---
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 #321: CB-12937 - (iOS) added new method handleOpenU...

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

    https://github.com/apache/cordova-ios/pull/321#discussion_r123258853
  
    --- Diff: CordovaLib/Classes/Public/CDVAppDelegate.m ---
    @@ -79,8 +79,16 @@ - (BOOL)application:(UIApplication*)application openURL:(NSURL*)url sourceApplic
             return NO;
         }
     
    +    NSDictionary *openURLData = @{
    +            @"url": url,
    +            @"sourceApplication": sourceApplication,
    +            @"annotation": annotation
    --- End diff --
    
    Good point @pbudzinsky .  I should revert to my original commit where I did explicit checks on sourceApplication and annotation then. sourceApplication can be nil too


---
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 issue #321: CB-12937 - (iOS) added new method handleOpenURLWithA...

Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on the issue:

    https://github.com/apache/cordova-ios/pull/321
  
    Slated for cordova-ios@4.5.0


---
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 #321: CB-12937 - (iOS) added new method handleOpenU...

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

    https://github.com/apache/cordova-ios/pull/321#discussion_r123074527
  
    --- Diff: CordovaLib/Classes/Public/CDVPlugin.m ---
    @@ -140,6 +142,36 @@ - (void)handleOpenURL:(NSNotification*)notification
         }
     }
     
    +/*
    +    NOTE: calls into JavaScript must not call or trigger any blocking UI, like alerts
    + */
    +- (void)handleOpenURLWithApplicationSourceAndAnnotation: (NSNotification*)notification
    +{
    +    
    +    // override to handle urls sent to your app
    +    // register your url schemes in your App-Info.plist
    +    
    +    // The notification object is an NSDictionary which contains
    +    // - url which is a type of NSURL
    +    // - sourceApplication which is a type of NSString and represents the package
    +    // id of the app that calls our app
    +    // - annotation which a type of Property list which can be several different types
    +    // please see https://developer.apple.com/library/content/documentation/General/Conceptual/DevPedia-CocoaCore/PropertyList.html
    +    
    +    NSDictionary*  notificationData = [notification object];
    +    
    +    if ([notificationData isKindOfClass: NSDictionary.class]){
    +        
    +        NSURL* url = notificationData[@"url"];
    +        NSString* sourceApplication = notificationData[@"sourceApplication"];
    --- End diff --
    
    I know it's trivial, but I would add the line where you extract the annotation here as well, to be complete, for users that rely on looking at source code on how to get at the data (who usually are not proficient at Obj-C)


---
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 #321: CB-12937 - (iOS) added new method handleOpenU...

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

    https://github.com/apache/cordova-ios/pull/321#discussion_r123253143
  
    --- Diff: CordovaLib/Classes/Public/CDVAppDelegate.m ---
    @@ -79,8 +79,16 @@ - (BOOL)application:(UIApplication*)application openURL:(NSURL*)url sourceApplic
             return NO;
         }
     
    +    NSDictionary *openURLData = @{
    +            @"url": url,
    +            @"sourceApplication": sourceApplication,
    +            @"annotation": annotation
    --- End diff --
    
    `annotation` could be `nil` here leading to unhandled exception resulting in a crash


---
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 issue #321: CB-12937 - (iOS) added new method handleOpenURLWithA...

Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:

    https://github.com/apache/cordova-ios/pull/321
  
    # [Codecov](https://codecov.io/gh/apache/cordova-ios/pull/321?src=pr&el=h1) Report
    > Merging [#321](https://codecov.io/gh/apache/cordova-ios/pull/321?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-ios/commit/4490abf273ec6d12810c8ff5ea16d197c58ecd4b?src=pr&el=desc) will **not change** coverage.
    > The diff coverage is `n/a`.
    
    [![Impacted file tree graph](https://codecov.io/gh/apache/cordova-ios/pull/321/graphs/tree.svg?width=650&src=pr&token=WomDD5jInz&height=150)](https://codecov.io/gh/apache/cordova-ios/pull/321?src=pr&el=tree)
    
    ```diff
    @@           Coverage Diff           @@
    ##           master     #321   +/-   ##
    =======================================
      Coverage   63.77%   63.77%           
    =======================================
      Files          14       14           
      Lines        1623     1623           
      Branches      277      277           
    =======================================
      Hits         1035     1035           
      Misses        588      588
    ```
    
    
    
    ------
    
    [Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-ios/pull/321?src=pr&el=continue).
    > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
    > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
    > Powered by [Codecov](https://codecov.io/gh/apache/cordova-ios/pull/321?src=pr&el=footer). Last update [4490abf...f49771b](https://codecov.io/gh/apache/cordova-ios/pull/321?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).



---
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