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 initialization it doesn't really hurt us to include
them.
However, the HTML5 elements are still missing from the list. I noticed
that you
are handling "embed" separately, but that means we are still holding
back to the
older design.
* There is another problem we are working on - We want some of the
element
attributes to be rewritten only when they meet certain criteria. Say
for
example, we want to rewrite a LINK HREF tag only when its TYPE is
"text/css" and
REL is "stylesheet" i.e. <LINK HREF="foo.css" TEXT="text/css" REL="stylesheet"></LINK> should be rewritten while <LINK
HREF="foo-zh.html"
HREFLANG="zh" REL="alternate></LINK> should not be rewritten. I am not very sure as to how this problem will be addressed with this
code
change.
* Lastly, we still want the application to have the flexibility of
configuring
what attributes of what elements should be rewritten. E.g. we
ourselves don't
want to rewrite any of html or flash resources, like IFRAME SRC or
EMBED SRC. I
don't know if providing a config file with a list of tagsToRewrite is
the best
approach.
To elaborate more, proxying a resource (like css , js) which cannot have javascript inside it is probably fine. But proxying an html page (text/html) through is error prone. If you have an <a href> tag for some webpage like this: <a href="www.example.org">hello</a> and we rewrite it to <a href="http://localhost:8080/gadgets/proxy?url=www.example.org";>hello</a> then we open up problems for xhr requests made by javascript included in the page. For example, a javascript on the page might be making an xmlHttpRequest for a relative url say "/foo" which earlier resolved to "www.example.org/foo" but will now resolve to "localhost:8080/foo" We found similar problems with flash as well. Hence the decision to not rewrite <iframe src> and <object> tags for now.
- Kuntal
http://codereview.appspot.com/1900042/show
