[ 
https://issues.apache.org/jira/browse/SHINDIG-1695?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13202411#comment-13202411
 ] 

[email protected] commented on SHINDIG-1695:
--------------------------------------------------------



bq.  On 2012-02-07 13:23:54, Stanton Sievers wrote:
bq.  > http://svn.apache.org/repos/asf/shindig/trunk/config/container.js, line 
144
bq.  > <https://reviews.apache.org/r/3768/diff/3/?file=72702#file72702line144>
bq.  >
bq.  >     Is this supposed 5MB or 5MiB? :)  You seem to have gone 5MiB.

MiB, sorry.  I'll fix the comment.


bq.  On 2012-02-07 13:23:54, Stanton Sievers wrote:
bq.  > 
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js,
 line 178
bq.  > <https://reviews.apache.org/r/3768/diff/3/?file=72704#file72704line178>
bq.  >
bq.  >     What's the reasoning behind doing this?  Not saying there's 
something wrong with it, but would like to understand the change.

Here's an explanation: 
http://www.scottlogic.co.uk/2010/10/javascript-array-performance/


bq.  On 2012-02-07 13:23:54, Stanton Sievers wrote:
bq.  > 
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js,
 line 1
bq.  > <https://reviews.apache.org/r/3768/diff/3/?file=72708#file72708line1>
bq.  >
bq.  >     Need boilerplate Apache header.  If Eclipse didn't add that for you 
automagically, we should look at updating the Eclipse IDE templates for .js 
files.

Ahh yes.  It did not.


bq.  On 2012-02-07 13:23:54, Stanton Sievers wrote:
bq.  > 
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js,
 line 19
bq.  > <https://reviews.apache.org/r/3768/diff/3/?file=72708#file72708line19>
bq.  >
bq.  >     It would be nice to comment that this is IE only (right?) so that it 
makes sense as people scan the code.

And FF 3.6


bq.  On 2012-02-07 13:23:54, Stanton Sievers wrote:
bq.  > 
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js,
 line 26
bq.  > <https://reviews.apache.org/r/3768/diff/3/?file=72708#file72708line26>
bq.  >
bq.  >     Do we know why appendChild() was being avoided in other places in 
the code where we setup the gadget iframes?

In that case we had a gadget site already in the DOM.   setting innerHTML 
wasn't hard because we already had a container element.
In this case, I can't set the innerHTML of the body tag, so I have to append 
the node.

I don't think it matters too much because it only ever happens once.


bq.  On 2012-02-07 13:23:54, Stanton Sievers wrote:
bq.  > 
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js,
 line 68
bq.  > <https://reviews.apache.org/r/3768/diff/3/?file=72708#file72708line68>
bq.  >
bq.  >     Should onprogress be optional if it's not going to work in all 
cases?  Maybe make it optional and also make it the last param.

Yes, I'll update the comment.
I don't think I'll be able to use the closure util if it needs to be last. :(


bq.  On 2012-02-07 13:23:54, Stanton Sievers wrote:
bq.  > 
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpRequest.java,
 line 194
bq.  > <https://reviews.apache.org/r/3768/diff/3/?file=72711#file72711line194>
bq.  >
bq.  >     Does it?  Looks like you're catching and logging.

Eclipse sneaked that in before when I wasn't.  Will update.


bq.  On 2012-02-07 13:23:54, Stanton Sievers wrote:
bq.  > 
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java,
 line 449
bq.  > <https://reviews.apache.org/r/3768/diff/3/?file=72712#file72712line449>
bq.  >
bq.  >     "The" not "There".... sorry. :)

Don't be, my turn will come to catch yours :)


bq.  On 2012-02-07 13:23:54, Stanton Sievers wrote:
bq.  > 
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java,
 line 472
bq.  > <https://reviews.apache.org/r/3768/diff/3/?file=72712#file72712line472>
bq.  >
bq.  >     I would make the max post size a constant somewhere.

I can if you want.   For something like this I don't think we need to clutter 
up the top of the class file, but I guess I don't care either way.
It should always be provided by the config, this is just a precaution.


- Dan


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


On 2012-02-07 02:31:39, Dan Dumont wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/3768/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-02-07 02:31:39)
bq.  
bq.  
bq.  Review request for Ryan Baxter and Stanton Sievers.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  With the desire to deprecate the makeRequest API, we still need a method 
to post files through the proxy. This new feature should not be bundled in with 
core, it should need to be explicitly requested. 
bq.  
bq.  It should allow modern browsers such as FF and Chrome to monitor form post 
progress. It should still be functional (do the post) for less capable browsers 
like IE, even though post progress may not be available.
bq.  
bq.  
bq.  This addresses bug SHINDIG-1695.
bq.      https://issues.apache.org/jira/browse/SHINDIG-1695
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestServletTest.java
 1241308 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java
 1241308 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/feature.xml
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/taming.js
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetException.java
 1241308 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpRequest.java
 1241308 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java
 1241308 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js
 1241308 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js
 1241308 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/features.txt
 1241308 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 
1241308 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/features/pom.xml 1241308 
bq.  
bq.  Diff: https://reviews.apache.org/r/3768/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Dan
bq.  
bq.


                
> new core-gadget feature "proxied-form-post" gadgets.proxiedMultipartFormPost
> ----------------------------------------------------------------------------
>
>                 Key: SHINDIG-1695
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1695
>             Project: Shindig
>          Issue Type: New Feature
>            Reporter: Dan Dumont
>            Assignee: Dan Dumont
>
> With the desire to deprecate the makeRequest API, we still need a method to 
> post files through the proxy.  This new feature should not be bundled in with 
> core, it should need to be explicitly requested.
> It should allow modern browsers such as FF and Chrome to monitor form post 
> progress.  It should still be functional (do the post) for less capable 
> browsers like IE, even though post progress may not be available.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to