John Ahlroos has posted comments on this change.

Change subject: Use JSON.parse() instead of eval() to deserialize rpc callback payload
......................................................................


Patch Set 4:

Discussed this offline with Artur for some time last Friday and I like the idea of setting the RPC version behind a property which application developers can set. It allows developers who knows they don't need to care about IE6/7, but needs to get this IE9 leak fixed asap, to just bump the version and start using JSON.parse() directly. It also allows us to wait with the decision of dropping IE6/7 completely until 2.6/3.0 without breaking any existing applications.

It also does mean developers which currently are supporting IE6-10 need to make a decision, either drop IE6/7 support and bump the version or live with the IE9 leak in eval(). Since we are anyway dropping support at some point it would be a good hint for those developers to start transitioning from IE6 to the modern world.

The way I thought of implementing this was to introduce a new constant SERIALIZATION_STREAM_MAX_VERSION (8) which would denote the maximum version supported by the server and SERIALIZATION_STREAM_VERSION would remain as 7 for now. We could also bump the SERIALIZATION_STREAM_MIN_VERSION to 7 if we so decide. I am still a bit confused over why the min version is 5 when the server still always is responding with the latest version.

The client would always use SERIALIZATION_STREAM_VERSION in the RPC request and the server would by default respond with SERIALIZATION_STREAM_VERSION (I would remove the version juggling currently implemented). If a custom version is set by the application developer via a property on the server then the server would respond with that instead. With version 8 the server would emit valid JSON while previous versions would emit what currently is emitted, hacks and all.

Once the client gets the response it would figure out which version is in use (by reading the payload as currently implemented, maybe moving the version out of the payload could be done in another changeset) and decode it with JSON.parse or eval(), whatever is appropriate for that version.

By doing it this way, this changeset does not grow to be a huge change and current behaviour is preserved for now at least. Once we get the decision to drop support for IE6/7 we just set SERIALIZATION_STREAM_VERSION=SERIALIZATION_STREAM_MAX_VERSION and everything should still work ok.

I don't have a good idea where I would put the property though. Should it be a System property as Artur suggests or should it instead be a setter in the RemoteServiceServlet? Ideas?

--
To view, visit https://gwt-review.googlesource.com/2900
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6062180397f5fabed1dd5f08140c2bd43a19fa9f
Gerrit-PatchSet: 4
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: John Ahlroos <j...@vaadin.com>
Gerrit-Reviewer: Artur Signell <ar...@vaadin.com>
Gerrit-Reviewer: Brian Slesinsky <skybr...@google.com>
Gerrit-Reviewer: Colin Alworth <niloc...@gmail.com>
Gerrit-Reviewer: John Ahlroos <j...@vaadin.com>
Gerrit-Reviewer: Leeroy Jenkins <jenk...@gwtproject.org>
Gerrit-Reviewer: Matthew Dempsky <mdemp...@google.com>
Gerrit-Reviewer: Thomas Broyer <t.bro...@gmail.com>
Gerrit-HasComments: No

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to