You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by riknoll <gi...@git.apache.org> on 2016/02/03 23:21:07 UTC

[GitHub] cordova-android pull request: CB-10498: Resume event should be sti...

GitHub user riknoll opened a pull request:

    https://github.com/apache/cordova-android/pull/257

    CB-10498: Resume event should be sticky if it has a plugin result

    Changes the resume event behavior as follows:
    
    1. When the app starts and there is no plugin result to be delivered, the **resume event will not fire** (same as current behavior)
    2. When the app is left and then resumed (e.g. press the home button and then return to the app), the **resume event will fire but will not be sticky** (same as current behavior)
    3. When the app is recreated after a plugin Activity finishes (i.e. the event payload contains a pending result), the **resume event will fire and will be sticky**

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

    $ git pull https://github.com/MSOpenTech/cordova-android CB-10498rebase

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

    https://github.com/apache/cordova-android/pull/257.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 #257
    
----
commit d7e111fb710c63c32ff21c15b6cb5c67ed6280cc
Author: riknoll <ri...@gmail.com>
Date:   2016-02-03T21:12:21Z

    CB-10498: Resume event should be sticky if it has a plugin result

----


---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257#discussion_r51923623
  
    --- Diff: cordova-js-src/platform.js ---
    @@ -58,6 +61,19 @@ module.exports = {
             bindButtonChannel('volumeup');
             bindButtonChannel('volumedown');
     
    +        // The resume event is not "sticky", but it is possible that the event
    +        // will contain the result of a plugin call. We need to ensure that the
    +        // plugin result is delivered even after the event is fired (CB-10498)
    +        var cordovaAddEventListener = document.addEventListener;
    --- End diff --
    
    Actually, it doesn't seem like Cordova events fire in the window...


---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257


---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257#issuecomment-180556020
  
    Sorry @csantanapr, I was dealing with some plugin fixes. I will merge this by the end of today; I need to test on a device.


---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257#issuecomment-181072343
  
    Any idea when this will be released?


---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257#discussion_r51911630
  
    --- Diff: cordova-js-src/platform.js ---
    @@ -58,6 +61,19 @@ module.exports = {
             bindButtonChannel('volumeup');
             bindButtonChannel('volumedown');
     
    +        // The resume event is not "sticky", but it is possible that the event
    +        // will contain the result of a plugin call. We need to ensure that the
    +        // plugin result is delivered even after the event is fired (CB-10498)
    +        var cordovaAddEventListener = document.addEventListener;
    --- End diff --
    
    @adamduren We already patch `addEventListener` in the common Cordova js code (hence the `cordovaAddEventListener` name). See https://github.com/apache/cordova-js/blob/master/src/cordova_b.js#L48
    
    Come to think of it, I should probably be doing the method in window as well. I'll update the PR. Thanks for taking a look!


---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257#issuecomment-180554647
  
    @riknoll you are going to merge this or there is more work to be done?


---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257#issuecomment-180171364
  
    @riknoll I tested the pull request and it's working for me.
    lastResume keeps the data, any new listener that get's attached get's the data. 



---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257#issuecomment-180126717
  
    @infil00p I'd appreciate it if you could review this one


---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257#issuecomment-180557444
  
    @riknoll sure no problem, I was just curious if there was more you wanted me to look at.



---
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-android pull request: CB-10498: Resume event should be sti...

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

    https://github.com/apache/cordova-android/pull/257#discussion_r51833834
  
    --- Diff: cordova-js-src/platform.js ---
    @@ -58,6 +61,19 @@ module.exports = {
             bindButtonChannel('volumeup');
             bindButtonChannel('volumedown');
     
    +        // The resume event is not "sticky", but it is possible that the event
    +        // will contain the result of a plugin call. We need to ensure that the
    +        // plugin result is delivered even after the event is fired (CB-10498)
    +        var cordovaAddEventListener = document.addEventListener;
    --- End diff --
    
    This is really the original document.addListener. Maybe the name should be changed to make that more clear.
    
    I think this is a simple and clean solution. However, I am a little weary about monkey patching `document.addEventListener`. I'm not sure what a better alternative would be though without adding unnecessary complexity.


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