You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Jason Chiang <ja...@gmail.com> on 2011/10/17 21:17:12 UTC

Review Request: Declarative action does not work on IE

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2404/
-----------------------------------------------------------

Review request for shindig, Ryan Baxter and Matthew Hatem.


Summary
-------

Declarative action does not work on IE.
1. The Declarative actions will be show undefined
2. Cannot run the actions

Link to jira https://issues.apache.org/jira/browse/SHINDIG-1642


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js 1185255 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1185255 

Diff: https://reviews.apache.org/r/2404/diff


Testing
-------

Test done on sample container with sample voip gadget on IE8, also passed jsUnit test.  


Thanks,

Jason


Re: Review Request: Declarative action does not work on IE

Posted by Ryan Baxter <rb...@gmail.com>.

> On 2011-10-20 00:13:57, Ryan Baxter wrote:
> > Committed revision 1186576.

Please close the review and the JIRA


- Ryan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2404/#review2687
-----------------------------------------------------------


On 2011-10-19 20:10:22, Jason Chiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2404/
> -----------------------------------------------------------
> 
> (Updated 2011-10-19 20:10:22)
> 
> 
> Review request for shindig, Ryan Baxter and Matthew Hatem.
> 
> 
> Summary
> -------
> 
> Declarative action does not work on IE.
> 1. The Declarative actions will be show undefined
> 2. Cannot run the actions
> 
> Link to jira https://issues.apache.org/jira/browse/SHINDIG-1642
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js 1186427 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1186427 
> 
> Diff: https://reviews.apache.org/r/2404/diff
> 
> 
> Testing
> -------
> 
> Test done on sample container with sample voip gadget on IE8, also passed jsUnit test.  
> 
> 
> Thanks,
> 
> Jason
> 
>


Re: Review Request: Declarative action does not work on IE

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2404/#review2687
-----------------------------------------------------------

Ship it!


Committed revision 1186576.

- Ryan


On 2011-10-19 20:10:22, Jason Chiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2404/
> -----------------------------------------------------------
> 
> (Updated 2011-10-19 20:10:22)
> 
> 
> Review request for shindig, Ryan Baxter and Matthew Hatem.
> 
> 
> Summary
> -------
> 
> Declarative action does not work on IE.
> 1. The Declarative actions will be show undefined
> 2. Cannot run the actions
> 
> Link to jira https://issues.apache.org/jira/browse/SHINDIG-1642
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js 1186427 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1186427 
> 
> Diff: https://reviews.apache.org/r/2404/diff
> 
> 
> Testing
> -------
> 
> Test done on sample container with sample voip gadget on IE8, also passed jsUnit test.  
> 
> 
> Thanks,
> 
> Jason
> 
>


Re: Review Request: Declarative action does not work on IE

Posted by Jason Chiang <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2404/
-----------------------------------------------------------

(Updated 2011-10-19 20:10:22.587736)


Review request for shindig, Ryan Baxter and Matthew Hatem.


Changes
-------

update patch after discussing with Dan

Thanks,
Jason


Summary
-------

Declarative action does not work on IE.
1. The Declarative actions will be show undefined
2. Cannot run the actions

Link to jira https://issues.apache.org/jira/browse/SHINDIG-1642


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js 1186427 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1186427 

Diff: https://reviews.apache.org/r/2404/diff


Testing
-------

Test done on sample container with sample voip gadget on IE8, also passed jsUnit test.  


Thanks,

Jason


Re: Review Request: Declarative action does not work on IE

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2404/#review2674
-----------------------------------------------------------



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js
<https://reviews.apache.org/r/2404/#comment6025>

    Just a nit, but I would do callback.apply(this, selectionObj ? [selectionObj] : []);



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js
<https://reviews.apache.org/r/2404/#comment6029>

    I wouldn't define a new function on the Object class, generally this isn't a good idea.



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js
<https://reviews.apache.org/r/2404/#comment6026>

    Don't iterate over array with for(var in object) syntax.  It can be dangerous.



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js
<https://reviews.apache.org/r/2404/#comment6032>

    for(var i = 0, actionObj; actionObj = actions[i]; i++) {
      var delkeys = [], n = 0;
      for (var key in actionObj) {
        actionObj[key.substring(1)] = actionObj[delkeys[n++] = key];
      }
      
      // Remove old keys.  Can't do this in previous loop because IE gets 
      // confused iterating over and modifying keys.
      for(var j = 0, key; key = delkeys[j]; j++) {
        delete actionObj[key];
      }
    }


- Dan


On 2011-10-19 01:17:48, Jason Chiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2404/
> -----------------------------------------------------------
> 
> (Updated 2011-10-19 01:17:48)
> 
> 
> Review request for shindig, Ryan Baxter and Matthew Hatem.
> 
> 
> Summary
> -------
> 
> Declarative action does not work on IE.
> 1. The Declarative actions will be show undefined
> 2. Cannot run the actions
> 
> Link to jira https://issues.apache.org/jira/browse/SHINDIG-1642
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1185865 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js 1185857 
> 
> Diff: https://reviews.apache.org/r/2404/diff
> 
> 
> Testing
> -------
> 
> Test done on sample container with sample voip gadget on IE8, also passed jsUnit test.  
> 
> 
> Thanks,
> 
> Jason
> 
>


Re: Review Request: Declarative action does not work on IE

Posted by Jason Chiang <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2404/
-----------------------------------------------------------

(Updated 2011-10-19 01:17:48.433263)


Review request for shindig, Ryan Baxter and Matthew Hatem.


Changes
-------

update patch based on feedback.

Thanks,
Jason


Summary
-------

Declarative action does not work on IE.
1. The Declarative actions will be show undefined
2. Cannot run the actions

Link to jira https://issues.apache.org/jira/browse/SHINDIG-1642


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1185865 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js 1185857 

Diff: https://reviews.apache.org/r/2404/diff


Testing
-------

Test done on sample container with sample voip gadget on IE8, also passed jsUnit test.  


Thanks,

Jason


Re: Review Request: Declarative action does not work on IE

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2404/#review2656
-----------------------------------------------------------



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js
<https://reviews.apache.org/r/2404/#comment5977>

    I was chatting with Dan about this and he says to try looping backwards in the array and changing the for loop to use the index instead of for...in


- Ryan


On 2011-10-18 15:32:30, Jason Chiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2404/
> -----------------------------------------------------------
> 
> (Updated 2011-10-18 15:32:30)
> 
> 
> Review request for shindig, Ryan Baxter and Matthew Hatem.
> 
> 
> Summary
> -------
> 
> Declarative action does not work on IE.
> 1. The Declarative actions will be show undefined
> 2. Cannot run the actions
> 
> Link to jira https://issues.apache.org/jira/browse/SHINDIG-1642
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js 1185711 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1185711 
> 
> Diff: https://reviews.apache.org/r/2404/diff
> 
> 
> Testing
> -------
> 
> Test done on sample container with sample voip gadget on IE8, also passed jsUnit test.  
> 
> 
> Thanks,
> 
> Jason
> 
>


Re: Review Request: Declarative action does not work on IE

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2404/#review2658
-----------------------------------------------------------



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js
<https://reviews.apache.org/r/2404/#comment5978>

    There's some whitespace at the end here, could you remove it?


- Dan


On 2011-10-18 15:32:30, Jason Chiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2404/
> -----------------------------------------------------------
> 
> (Updated 2011-10-18 15:32:30)
> 
> 
> Review request for shindig, Ryan Baxter and Matthew Hatem.
> 
> 
> Summary
> -------
> 
> Declarative action does not work on IE.
> 1. The Declarative actions will be show undefined
> 2. Cannot run the actions
> 
> Link to jira https://issues.apache.org/jira/browse/SHINDIG-1642
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js 1185711 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1185711 
> 
> Diff: https://reviews.apache.org/r/2404/diff
> 
> 
> Testing
> -------
> 
> Test done on sample container with sample voip gadget on IE8, also passed jsUnit test.  
> 
> 
> Thanks,
> 
> Jason
> 
>


Re: Review Request: Declarative action does not work on IE

Posted by Jason Chiang <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2404/
-----------------------------------------------------------

(Updated 2011-10-18 15:32:30.415318)


Review request for shindig, Ryan Baxter and Matthew Hatem.


Changes
-------

Update a new patch, previous patch is a wrong fix. 


Summary
-------

Declarative action does not work on IE.
1. The Declarative actions will be show undefined
2. Cannot run the actions

Link to jira https://issues.apache.org/jira/browse/SHINDIG-1642


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions.js 1185711 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1185711 

Diff: https://reviews.apache.org/r/2404/diff


Testing
-------

Test done on sample container with sample voip gadget on IE8, also passed jsUnit test.  


Thanks,

Jason