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.