[
https://issues.apache.org/jira/browse/SHINDIG-1695?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13202363#comment-13202363
]
[email protected] 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
