[
https://issues.apache.org/jira/browse/JSPWIKI-566?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14286228#comment-14286228
]
brushed commented on JSPWIKI-566:
---------------------------------
Hi David,
Thx for large rewrite.
I haven't been able to test it, but have just read through the code. (mainly
looking at the client-side part)
Would it be possible to update your test-site with the latest code base ?
Few quick comments / questions from my review :
- The JS introduces some global functions like ajaxHtmlCall(...),
ajaxJSONCall(...), .. and more.
Consistent with the rest of the JS, I'd suggest you put those inside the Wiki
namespace,
so the calls become Wiki.ajaxHtmlCall(...) , Wiki.ajaxJSONCall(...) etc.
- I noticed the ajax mechanism just passes one parameter which is coded inside
the URL.
The current json-rpc implementations passes an array of parameters via a JSON
object.
Many of the rpc calls currently use 2 parameters -- the 2nd parameter has now
become hard-coded.
I think passing the ajax request parameter in the URL should be avoided.
The current json-rpc implementation uses an elegant and simple
approach by passing a json object in the FORM-DATA of the ajax request.
It probably will make the url path parsing easier too.
- The url path is hard-coded in both the JAVA and the JAVA-SCRIPT.
I suggest to make the "/ajax" path a variable; and pass it to the client in
a META tag.
Check out commonheader.jsp how this can be done in a simple way;
and the Wiki JS will automatically read it at page load.
- The xmlHttpRequest handling code is a duplicate from the javascript
library.
This can be simplified by using new Request.JSON(...) or new
Request.HTML(...) ;
which also provides good support for error handling.
- The ajax JS error handling many need some extra testing : it seems to return
an error-msg string, which will be handled as a normal result by the
call-back function.
The current code returns an object with either a result property or an error
property.
- Also consider to add/keep a call to .stripScripts() to protect against xss
vulnerabilities.
- I noticed this code change -- was this intentional ?
{noformat}
- if(euro) euro = v.test(/^[£$€][0-9.,]+/);
+ if(euro) euro = v.test(/^[£$€][0-9.,]+/);
{noformat}
Br,
dirk
> AJAX server-side rewrite
> ------------------------
>
> Key: JSPWIKI-566
> URL: https://issues.apache.org/jira/browse/JSPWIKI-566
> Project: JSPWiki
> Issue Type: Improvement
> Components: Core & storage
> Affects Versions: 2.10.1
> Reporter: Janne Jalkanen
> Assignee: David Vittor
> Attachments: ajaxDispatchServlet.patch, ajaxFunctions.patch,
> ajaxFunctions.patch, test.html
>
>
> The AJAX library we're currently using is a bit problematic, as it stores
> non-serializable stuff in the HttpSession (causing all sorts of nasty
> exception reports in default configurations of Tomcat, and preventing
> clustering). It does provide a very nice, reflection-based interface so that
> we can expose any class/method as a JSON endpoint, but this does not really
> work well with our auth system.
> We should replace the jabsorb stuff with a Stripes-native solution (possibly
> with some extensions to allow particular beans to expose methods as if we
> were using jabsorb).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
