shindig-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SHINDIG-1695) new core-gadget feature "proxied-form-post" gadgets.proxiedMultipartFormPost
Date Tue, 07 Feb 2012 13:25:00 GMT

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

jiraposter@reviews.apache.org commented on SHINDIG-1695:
--------------------------------------------------------


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


LGTM for the most part.  Kudos on killing whitespace like it's your job. :)


http://svn.apache.org/repos/asf/shindig/trunk/config/container.js
<https://reviews.apache.org/r/3768/#comment10666>

    Is this supposed 5MB or 5MiB? :)  You seem to have gone 5MiB.  



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js
<https://reviews.apache.org/r/3768/#comment10667>

    What's the reasoning behind doing this?  Not saying there's something wrong with it, but
would like to understand the change.



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js
<https://reviews.apache.org/r/3768/#comment10668>

    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.



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js
<https://reviews.apache.org/r/3768/#comment10670>

    It would be nice to comment that this is IE only (right?) so that it makes sense as people
scan the code.



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js
<https://reviews.apache.org/r/3768/#comment10669>

    Do we know why appendChild() was being avoided in other places in the code where we setup
the gadget iframes?



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js
<https://reviews.apache.org/r/3768/#comment10671>

    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.



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpRequest.java
<https://reviews.apache.org/r/3768/#comment10672>

    Does it?  Looks like you're catching and logging.



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java
<https://reviews.apache.org/r/3768/#comment10673>

    "The" not "There".... sorry. :)



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java
<https://reviews.apache.org/r/3768/#comment10674>

    I would make the max post size a constant somewhere.


- Stanton


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

        

Mime
View raw message