Adding host and proxy path for Proxy Uris. (issue1697041)

Reviewers: johnfargo, zhoresh, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, Message: Closing issue as this has been submitted in http://codereview.appspot.com/1686043/show Description: 1) Adding host and path for proxyied uris without which proxy and concat servlets dont seem t

Re: Adding AbsolutePathReferenceRewriter (issue1699041)

Reviewers: johnfargo, zhoresh, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, Message: On 2010/06/17 19:40:12, zhoresh wrote: Lgtm, but do you really need the new rewriter class? You can just instantiate the DomWalker.Rewriter with the visitor in the constructor. As discussed

Re: Adding AbsolutePathReferenceRewriter (issue1699041)

On 2010/06/17 19:59:55, gagan.goku wrote: On 2010/06/17 19:40:12, zhoresh wrote: > Lgtm, but do you really need the new rewriter class? > You can just instantiate the DomWalker.Rewriter with the visitor in the > constructor. As discussed, keeping the rewriter class for consistency, but makin

Re: Setting timeout for gargoyle webClient so that build does not hang. (issue1686045)

Reviewers: johnfargo, zhoresh, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, Message: On 2010/06/17 15:37:23, zhoresh wrote: lgtm, committed (r955437) Thanks. Description: Currently build hangs when webclient is not able to fetch a resource from local server as part of EndtoE

Adding capability in AbsolutePathReferenceVisitor to resolve relative to base tag url (issue1726041)

Reviewers: johnfargo, zhoresh, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, Message: Synced to r955719. Description: Adding capability in AbsolutePathReferenceVisitor to resolve relative to base tag url Base tag url looks like this: http://www.google.com/";> And then any reso

Re: Adding capability in AbsolutePathReferenceVisitor to resolve relative to base tag url (issue1726041)

http://codereview.appspot.com/1726041/diff/6001/7002 File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java (right): http://codereview.appspot.com/1726041/diff/6001/7002#newcode42 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/Absolute

Re: Adding host and proxy path for Proxy Uris. (issue1697041)

http://codereview.appspot.com/1697041/diff/1/2 File config/container.js (right): http://codereview.appspot.com/1697041/diff/1/2#newcode95 config/container.js:95: "defaultShindigTestHost": "http://localhost:9003";, On 2010/06/17 21:30:14, johnfargo wrote: why not just remove http:// from this UR

Re: Adding capability in AbsolutePathReferenceVisitor to resolve relative to base tag url (issue1726041)

http://codereview.appspot.com/1726041/diff/6001/7002 File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java (right): http://codereview.appspot.com/1726041/diff/6001/7002#newcode171 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/Absolut

Re: Adding capability in AbsolutePathReferenceVisitor to resolve relative to base tag url (issue1726041)

This time really Done. http://codereview.appspot.com/1726041/diff/17001/18001 File java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitorTest.java (right): http://codereview.appspot.com/1726041/diff/17001/18001#newcode142 java/gadgets/src/test/java/org/apache

Re: Adding capability in AbsolutePathReferenceVisitor to resolve relative to base tag url (issue1726041)

On 2010/06/17 23:04:30, johnfargo wrote: Committed. On Thu, Jun 17, 2010 at 3:00 PM, wrote: > This time really Done. > > > > http://codereview.appspot.com/1726041/diff/17001/18001 > File > > java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/Absolu

Adding capability in accel servlet to act as accelerating proxy (issue1712043)

Reviewers: zhoresh, johnfargo, dev-remailer_shindig.apache.org, shindig.remailer_gmail.com, Message: Please review. Description: - Adding accel servlet as catch all servlet - Modifying AccelUriManager to deal with all urls. Please review this at http://codereview.appspot.com/1712043/show Af

Re: Adding AbsolutePathReferenceRewriter and DomainBalancingUriRe (issue1674041)

Please review. http://codereview.appspot.com/1674041/show

Re: Adding BaseTagRemoverRewriter (issue1630042)

On 2010/06/13 06:04:23, gagan.goku wrote: http://codereview.appspot.com/1630042/diff/1/3 File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/BaseTagRemoverRewriter.java (right): http://codereview.appspot.com/1630042/diff/1/3#newcode33 java/gadgets/src/main/java/org/apache/s

Re: Supporting http proxy in basic http fetcher (issue1635042)

On 2010/06/15 23:17:13, gagan.goku wrote: On 2010/06/15 06:52:20, Paul Lindner wrote: > I think you can do it like this: > > ConnRouteParams.setDefaultProxy(params, new HttpHost(host,port, scheme)); > This worked perfectly. Thanks > (have not tested it, but looks like the proper way to do th

Escaping non-ascii values (issue1693042)

Reviewers: johnfargo, zhoresh, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, Message: Please review. Description: Non ascii strings encoded in utf8 cause problems in some editors like intellij. Escaping it non-ascii content to satisfy intellij. Please review this at http://code

Re: Supporting http proxy in basic http fetcher (issue1635042)

On 2010/06/21 18:18:58, Paul Lindner wrote: can you please retain the previous method of setting the proxy from environment variables? It can be used when there is no proxy set via guice. I know there are some people using that feature and removing this would break them. Otherwise looks

Re: Escaping non-ascii values (issue1693042)

On 2010/06/21 17:50:01, Paul Lindner wrote: lgtm, committed Thanks :) http://codereview.appspot.com/1693042/show

Re: Supporting http proxy in basic http fetcher (issue1635042)

On 2010/06/21 20:04:46, Paul Lindner wrote: yes please. Done. On Mon, Jun 21, 2010 at 1:01 PM, wrote: > On 2010/06/21 18:18:58, Paul Lindner wrote: > >> can you please retain the previous method of setting the proxy from >> > environment > >> variables? It ca

Re: Converting private fields to protected for ease of extending (issue1705043)

Reviewers: johnfargo, zhoresh, dev-remailer_shindig.apache.org, shindig.remailer_gmail.com, Message: On 2010/06/21 20:51:03, zhoresh wrote: http://codereview.appspot.com/1705043/diff/1/2 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java (right): http://

Re: Converting private fields to protected for ease of extending (issue1705043)

http://codereview.appspot.com/1705043/diff/1/2 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java (right): http://codereview.appspot.com/1705043/diff/1/2#newcode59 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:59: protected Acce

Re: Converting private fields to protected for ease of extending (issue1705043)

On 2010/06/21 21:09:27, zhoresh wrote: lgtm, committed (r956695) Thanks http://codereview.appspot.com/1705043/show

Re: Supporting http proxy in basic http fetcher (issue1635042)

On 2010/06/21 23:07:44, Paul Lindner wrote: thanks! committed. thank you http://codereview.appspot.com/1635042/show

Re: Adding BaseTagRemoverRewriter (issue1630042)

On 2010/06/21 21:53:16, zhoresh wrote: lgtm, I removed @Override annotations and committed (r956714) Thanks. http://codereview.appspot.com/1630042/show

Re: Adding capability in accel servlet to act as accelerating proxy (issue1712043)

http://codereview.appspot.com/1712043/diff/5001/6003 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java (right): http://codereview.appspot.com/1712043/diff/5001/6003#newcode123 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:123:

Re: Adding capability in accel servlet to act as accelerating proxy (issue1712043)

Done. On 2010/06/22 02:31:12, fargo wrote: On Mon, Jun 21, 2010 at 7:11 PM, wrote: > > http://codereview.appspot.com/1712043/diff/5001/6003 > File > > java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java > (right): > > http://coderev

Re: Adding capability in accel servlet to act as accelerating proxy (issue1712043)

http://codereview.appspot.com/1712043/diff/19001/20005 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/AccelUriManager.java (right): http://codereview.appspot.com/1712043/diff/19001/20005#newcode32 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/AccelUriManager.java:32:

Re: Adding AbsolutePathReferenceRewriter and DomainBalancingUriRe (issue1674041)

On 2010/06/21 19:16:34, johnfargo wrote: Hi Gagan: I'm a little confused still at the primary intended action of this CL. Is the idea to balance domains that _aren't_ controlled by the Shindig server (ie. not proxy URLs) but which _are_ known to be balance-able in some fashion? This type

Re: Adding AbsolutePathReferenceRewriter and DomainBalancingUriRe (issue1674041)

Please ignore changes in this file as being correctly modified as http://codereview.appspot.com/1712043/diff/7002/32007 http://codereview.appspot.com/1674041/show

Re: Adding AbsolutePathReferenceRewriter and DomainBalancingUriRe (issue1674041)

On 2010/06/22 14:51:19, gagan.goku wrote: Please ignore changes in this file as being correctly modified as http://codereview.appspot.com/1712043/diff/7002/32007 I meant ignore changes in files AccelUriManager.java http://codereview.appspot.com/1674041/show

Re: Adding AbsolutePathReferenceRewriter and DomainBalancingUriRe (issue1674041)

On 2010/06/22 14:51:48, gagan.goku wrote: On 2010/06/22 14:51:19, gagan.goku wrote: > Please ignore changes in this file as being correctly modified as > http://codereview.appspot.com/1712043/diff/7002/32007 I meant ignore changes in files AccelUriManager.java I added DefaultProxyUriManager

Re: Adding capability in accel servlet to act as accelerating proxy (issue1712043)

On 2010/06/22 23:23:55, johnfargo wrote: (I made this change myself before committing) http://codereview.appspot.com/1712043/diff/7002/32004 File java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultAccelUriManagerTest.java (right): http://codereview.appspot.com/1712043/diff/

Re: Change declaration of ProxyBase#setRequestHeaders to throw GadgetException (issue1683050)

On 2010/06/23 16:05:22, janluehe wrote: Do you want to wrap an inner exception into GadgetException? Currently there doesn't seem to be anything in setRequestHeaders that can throw an exception. If you have a case where some null pointer exception etc is returned, then this change makes sense.

Re: Change declaration of ProxyBase#setRequestHeaders to throw GadgetException (issue1683050)

On 2010/06/23 16:55:47, janluehe wrote: We override setRequestHeaders to add some product-specific headers, and when determining their values, checked exceptions may be thrown. Currently, we have to log them as severe errors, but we'd really like to be able to wrap them inside GadgetExcepti

Re: Adding DomainBalancingUriRe (issue1674041)

Please review. http://codereview.appspot.com/1674041/show

Cleaning up HtmlAccelServlet (issue1798042)

Reviewers: johnfargo, zhoresh, cool-shindig-committers_googlegroups.com, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, Please review this at http://codereview.appspot.com/1798042/show Affected files: main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java main/j

Re: Refactoring servlets to allow caching response (issue1770043)

fixing tests http://codereview.appspot.com/1770043/show

Re: Refactoring servlets to allow caching response (issue1770043)

fixing tests http://codereview.appspot.com/1770043/show

Re: Cleaning up HtmlAccelServlet (issue1798042)

fixing bad things http://codereview.appspot.com/1798042/show

Returning same status code as fetched (issue1811042)

Reviewers: johnfargo, zhoresh, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, cool-shindig-committers_googlegroups.com, Description: Returning the error http status code as the fetched resource. Since we want to proxy the resource through, we want the user to see the exact code re

Re: Returning same status code as fetched (issue1811042)

fixing tests http://codereview.appspot.com/1811042/show

Re: Returning same status code as fetched (issue1811042)

On 2010/07/12 09:27:19, gagan.goku wrote: fixing tests Comment addressed. http://codereview.appspot.com/1811042/show

Enabling StyleTagProxyEmbeddedUrlsRewriter for accel (issue1813041)

Reviewers: johnfargo, zhoresh, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, cool-shindig-committers_googlegroups.com, Please review this at http://codereview.appspot.com/1813041/show Affected files: RewriteModule.java Index: RewriteModule.java

Handling post requests for Accel servlet (issue1822041)

Reviewers: johnfargo, zhoresh, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, cool-shindig-committers_googlegroups.com, Please review this at http://codereview.appspot.com/1822041/show Affected files: main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java main/java

Re: Handling post requests for Accel servlet (issue1822041)

adding tests http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

beautification http://codereview.appspot.com/1822041/show

Re: Returning same status code as fetched (issue1811042)

http://codereview.appspot.com/1811042/diff/6001/7001 File main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java (right): http://codereview.appspot.com/1811042/diff/6001/7001#newcode89 main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:89: // In case of errors where we want t

Re: Returning same status code as fetched (issue1811042)

addressing comments http://codereview.appspot.com/1811042/show

Re: Handling post requests for Accel servlet (issue1822041)

http://codereview.appspot.com/1822041/diff/9001/10004 File main/java/org/apache/shindig/gadgets/uri/UriUtils.java (right): http://codereview.appspot.com/1822041/diff/9001/10004#newcode70 main/java/org/apache/shindig/gadgets/uri/UriUtils.java:70: // Headers that the fetcher itself would like to f

Re: Handling post requests for Accel servlet (issue1822041)

addressing comments http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

reverting basichttpfetcher http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

fixing comment http://codereview.appspot.com/1822041/show

Re: [SHINDIG-1382] Disable HttpClient cookie handling (issue1825042)

On 2010/07/13 18:17:56, mat.mannion wrote: Nice fix :) Would be sad if ppl start using shindig as a proxy server, only to realize later that your cookies are not yours anymore. http://codereview.appspot.com/1825042/show

Re: Returning same status code as fetched (issue1811042)

addressing comments http://codereview.appspot.com/1811042/show

Re: Returning same status code as fetched (issue1811042)

http://codereview.appspot.com/1811042/diff/16001/17001 File main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java (right): http://codereview.appspot.com/1811042/diff/16001/17001#newcode88 main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:88: if (errorResponse != null) { On

Re: Handling post requests for Accel servlet (issue1822041)

setting follow redirect to false for accel http://codereview.appspot.com/1822041/show

Re: Refactoring servlets to allow caching response (issue1770043)

addressing comments http://codereview.appspot.com/1770043/show

Re: Refactoring servlets to allow caching response (issue1770043)

http://codereview.appspot.com/1770043/diff/13001/14001 File main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java (left): http://codereview.appspot.com/1770043/diff/13001/14001#oldcode211 main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:211: throws IOException { On 2010/07

Re: Refactoring servlets to allow caching response (issue1770043)

fixing bad things http://codereview.appspot.com/1770043/show

Re: Cleaning up HtmlAccelServlet (issue1798042)

addressing comments http://codereview.appspot.com/1798042/show

Re: Cleaning up HtmlAccelServlet (issue1798042)

addressing comments http://codereview.appspot.com/1798042/show

Re: Cleaning up HtmlAccelServlet (issue1798042)

http://codereview.appspot.com/1798042/diff/7001/7 File main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java (left): http://codereview.appspot.com/1798042/diff/7001/7#oldcode72 main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java:72: } On 2010/07/14 04:51:10, vikaas.ar

Re: Enabling StyleTagProxyEmbeddedUrlsRewriter for accel (issue1813041)

On 2010/07/14 20:48:57, johnfargo wrote: LGTM, committed. On 2010/07/12 11:05:02, gagan.goku wrote: > Thanks http://codereview.appspot.com/1813041/show

Re: Returning same status code as fetched (issue1811042)

addressing comments http://codereview.appspot.com/1811042/show

Re: Returning same status code as fetched (issue1811042)

reverting ProxyingContentRewriter http://codereview.appspot.com/1811042/show

Re: Returning same status code as fetched (issue1811042)

On 2010/07/15 05:27:09, gagan.goku wrote: reverting ProxyingContentRewriter Comments addressed. And to emphasize how important http status codes are, Rietveld is throwing "500 Server Error" when i try to Publish+Mail comments. http://codereview.appspot.com/1811042/show

Re: Handling post requests for Accel servlet (issue1822041)

http://codereview.appspot.com/1822041/diff/33001/34001 File main/java/org/apache/shindig/gadgets/http/HttpRequest.java (right): http://codereview.appspot.com/1822041/diff/33001/34001#newcode54 main/java/org/apache/shindig/gadgets/http/HttpRequest.java:54: // TODO: Convert to Map> to allow multip

Re: Handling post requests for Accel servlet (issue1822041)

addressing comments http://codereview.appspot.com/1822041/show

Re: Returning same status code as fetched (issue1811042)

On 2010/07/15 05:29:40, gagan.goku wrote: On 2010/07/15 05:27:09, gagan.goku wrote: > reverting ProxyingContentRewriter Comments addressed. And to emphasize how important http status codes are, Rietveld is throwing "500 Server Error" when i try to Publish+Mail comments. On 2010/07/14 20:45

Re: Adding DomainBalancingUriRe (issue1674041)

Hi John Addressed your comments. Please take another look. For now, i have not bound ProxyUriManager to DomainBalancingProxyUriManager, though i did test it with 0.localhost and 1.localhost and it seemed to be generating the desired html. If possible, I would like to take that up in the following

Re: Embed and object tags should not be rewritten, input src and body background should be rewritten (issue1806044)

Ditto. This change is in good enough shape to send to shindig dev@ Ps: Sorry for the late reply :( http://codereview.appspot.com/1806044/show

Re: Handling post requests for Accel servlet (issue1822041)

But there could be multiple header values for the same header, like: Set-Cookie: NAME=value; domain=hello Set-Cookie: NAME2=value2; domain=buffalo Set header would just retain the last one (i hope thats why HttpServletResponse has addHeader and setHeader functions) Agreed w/ the rationale;

Re: Handling post requests for Accel servlet (issue1822041)

addressing comments http://codereview.appspot.com/1822041/show

Re: Adding ProxyingRewriter which will only proxy resources (issue1711053)

On 2010/07/18 04:43:41, anupama.dutta wrote: LGTM. thanks :) http://codereview.appspot.com/1711053/show

Re: Adding ProxyingRewriter which will only proxy resources (issue1711053)

really addressing comments http://codereview.appspot.com/1711053/show

Re: Adding ProxyingRewriter which will only proxy resources (issue1711053)

really addressing comments http://codereview.appspot.com/1711053/show

Re: shindig.uri library (issue1875044)

http://codereview.appspot.com/1875044/diff/2001/3005 File features/src/main/javascript/features/shindig.uri/uri.js (right): http://codereview.appspot.com/1875044/diff/2001/3005#newcode60 features/src/main/javascript/features/shindig.uri/uri.js:60: function parseFrom(url) { On 2010/07/21 06:41:44

Re: Handling NullPointerException correctly in GadgetHtmlParser (issue1871046)

Please review. http://codereview.appspot.com/1871046/show

Fixing small error in PropertiesModule (issue1883042)

Reviewers: dev-remailer_shindig.apache.org, Please review this at http://codereview.appspot.com/1883042/show Affected files: java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java Index: java/common/src/main/java/org/apache/shindig/common/PropertiesModule.java ===

Re: Support JS-mode from proxy service, returning dataUri. (issue1696056)

Nice change :) Small nits: http://codereview.appspot.com/1696056/diff/1/3 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (left): http://codereview.appspot.com/1696056/diff/1/3#oldcode140 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHan

Re: Returning same status code as that of the fetched resource (issue1811042)

On 2010/07/26 12:16:10, anupama.dutta wrote: LGTM. http://codereview.appspot.com/1811042/diff/37001/38001 File common/conf/shindig.properties (right): http://codereview.appspot.com/1811042/diff/37001/38001#newcode145 common/conf/shindig.properties:145: # Remap internal server errors receiv

Re: Refactoring ProxyHandler to also use UriUtils methods (issue1855044)

http://codereview.appspot.com/1855044/diff/45001/46005 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (right): http://codereview.appspot.com/1855044/diff/45001/46005#newcode154 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:1

Re: Handling NullPointerException correctly in GadgetHtmlParser.parseDom() (issue1871046)

On 2010/07/26 18:36:08, johnfargo wrote: As discussed independently, this is definitely a bit of a kluge to accommodate problems that exist in specific configurations of execution rather than being well-encapsulated. Even so, I'll commit this with a TODO to consider removal, in order to imp

Re: Refactoring ProxyHandler to also use UriUtils methods (issue1855044)

Thanks for the review Ziv. http://codereview.appspot.com/1855044/diff/45001/46004 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java (left): http://codereview.appspot.com/1855044/diff/45001/46004#oldcode60 java/gadgets/src/main/java/org/apache/shindig/gadgets/serv

Re: Handle Bad header in proxy (issue1867046)

lgtm http://codereview.appspot.com/1867046/diff/1/2 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (right): http://codereview.appspot.com/1867046/diff/1/2#newcode146 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:146: // Ski

Re: Fixing NPE because of EOFException.getMessage returning null (issue1872044)

On 2010/07/26 22:15:23, johnfargo wrote: FYI committed last week, as r966159. http://svn.apache.org/viewvc?view=revision&revision=966159 Hi John These are actually 2 related but different bugs. You have checked in Vikas's bug which is a known GZIP inflater bug. This bug is another bug in GZ

Re: Handle Bad header in proxy (issue1867046)

On 2010/07/27 20:09:03, zhoresh wrote: On 2010/07/27 05:36:14, gagan.goku wrote: > lgtm > > http://codereview.appspot.com/1867046/diff/1/2 > File > java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java > (right): > > http://codereview.appspot.com/1867046/diff/1/2#newc

Re: Use full attribute list for rewriting URLs (issue1900042)

On 2010/07/28 12:02:21, Kuntal Loya wrote: Hi Jasvir, This code looks great. I however have a couple of doubts - * The tags you mentioned in the list contain a lot of elements-attribute combination which are no longer supported by the current browsers. Given that this is a one time init

Re: Refactoring ProxyHandler to also use UriUtils methods (issue1855044)

http://codereview.appspot.com/1855044/diff/58001/55007 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (right): http://codereview.appspot.com/1855044/diff/58001/55007#newcode69 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:69

Re: [BugFix] Modifying JsonContainerConfig to add SERVER_HOST and SERVER_PORT env. to all containers (issue2000043)

I believe Jesse raised this problem sometime ago: http://markmail.org/message/27jfrktjrmhw7nm5 http://codereview.appspot.com/243/diff/13001/5003 File java/common/src/main/java/org/apache/shindig/config/JsonContainerConfig.java (right): http://codereview.appspot.com/243/diff/13001/5003#

Re: [BugFix] Modifying JsonContainerConfig to add SERVER_HOST and SERVER_PORT env. to all containers (issue2000043)

Thanks for taking a look Paul. Comment inline: http://codereview.appspot.com/243/diff/13001/5003 File java/common/src/main/java/org/apache/shindig/config/JsonContainerConfig.java (left): http://codereview.appspot.com/243/diff/13001/5003#oldcode378 java/common/src/main/java/org/apache/sh

Re: Vanilla Caja html parser (issue2006042)

Thanks for the thorough review Anupama. http://codereview.appspot.com/2006042/diff/20002/21007 File java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/caja/VanillaCajaHtmlParserTest.java (right): http://codereview.appspot.com/2006042/diff/20002/21007#newcode56 java/gadgets/src/test/jav

Re: Refactoring ProxyingVisitor so that its functionality can be extended by other visitors (issue1949051)

http://codereview.appspot.com/1949051/diff/5001/6001 File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java (left): http://codereview.appspot.com/1949051/diff/5001/6001#oldcode85 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java:8

Re: Refactoring ProxyingVisitor so that its functionality can be extended by other visitors (issue1949051)

Thanks Paul http://codereview.appspot.com/1949051/

Re: Adding gagan as a shindig committer :) (issue2106044)

http://codereview.appspot.com/2106044/

Re: Adding gagan as a shindig committer :) (issue2106044)

http://codereview.appspot.com/2106044/

Re: Adding gagan as a shindig committer :) (issue2106044)

Committed. Since no1 has sent me any hate / die mail yet :) , i think things are going to be okay. Thanks to every1 for helping out. On 2010/09/02 19:53:16, gagan.goku wrote: It worked :) Please take a look now. On Thu, Sep 2, 2010 at 9:31 AM, wrote: > LGTM!

Re: Vanilla Caja html parser (issue2006042)

http://codereview.appspot.com/2006042/

Re: Vanilla Caja html parser (issue2006042)

http://codereview.appspot.com/2006042/diff/37001/38005 File java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/VanillaCajaHtmlParser.java (right): http://codereview.appspot.com/2006042/diff/37001/38005#newcode70 java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/Vanil

Re: Vanilla Caja html parser (issue2006042)

On 2010/09/13 20:15:55, jasvir wrote: LGTM Thanks for the valuable review Jasvir :) http://codereview.appspot.com/2006042/

Re: Vanilla Caja html parser (issue2006042)

On 2010/09/13 20:17:12, gagan.goku wrote: On 2010/09/13 20:15:55, jasvir wrote: > LGTM Thanks for the valuable review Jasvir :) Ran mvn -e clean install. Build looks awesome. Committed as r997836. http://codereview.appspot.com/2006042/

  1   2   >