Details

Description

Currently events added inside onclick, onsubmit attributes are problematic because they cannot be cancelled by other event handlers that are attached to events via javascript's addEventListener() and friends. We should extract all such inlined handlers and attach them via our Wicket.Event api.

Martin Grigorov
added a comment - 18/Nov/11 14:39 We need to improve HeaderResponse to not create a new <script> for each event binding. It would be much better if there is just one <script> with bigger body.

Martin Grigorov
added a comment - 18/Nov/11 14:44 How big will be the refactoring here ?
Matej did some major refactoring at http://svn.apache.org/repos/asf/wicket/sandbox/knopp/experimental few years ago. See https://cwiki.apache.org/WICKET/wicket-ajax-rewriting.html for more info. While I like the approach with o.a.w.ajax.AjaxRequestAttributes.java there I prefer to avoid of making bigger changes. Knowing how much Ajax I used in my previous applications at dailyjob I see it will be quite a work to migrate.

doing it this way wont solve the problem of not being able to attach multiple behaviors to the same event. we should probably write out the entire handler in wicket.event.add() instead of writing it into the attribute.

also, i think adding AjaxAttributes now would be a Good Thing. they stay out of the way unless you want to touch them. and, if you are touching the callback url in the behavior chances are you will want to rewrite your code using attributes because without them it is very fragile. you have to do a sql-injeciton-like attack on the url string to even get your own attributes into it...so the current way is broken anyways.

Igor Vaynberg
added a comment - 18/Nov/11 16:17 doing it this way wont solve the problem of not being able to attach multiple behaviors to the same event. we should probably write out the entire handler in wicket.event.add() instead of writing it into the attribute.
also, i think adding AjaxAttributes now would be a Good Thing. they stay out of the way unless you want to touch them. and, if you are touching the callback url in the behavior chances are you will want to rewrite your code using attributes because without them it is very fragile. you have to do a sql-injeciton-like attack on the url string to even get your own attributes into it...so the current way is broken anyways.

Issuing several Wicket.Event.add(targetX, 'click', fn1), Wicket.Event.add(targetX, 'click', fn2), Wicket.Event.add(targetX, 'click', fn3), ... will execute all functions, not just the last one.
I see WicketTester as a problem here because it assumes that only one behavior per event type can exists. I'll create a task ticket for this functionality to make sure it is properly handled.

Martin Grigorov
added a comment - 21/Nov/11 12:25 Issuing several Wicket.Event.add(targetX, 'click', fn1), Wicket.Event.add(targetX, 'click', fn2), Wicket.Event.add(targetX, 'click', fn3), ... will execute all functions, not just the last one.
I see WicketTester as a problem here because it assumes that only one behavior per event type can exists. I'll create a task ticket for this functionality to make sure it is properly handled.
OK. I'm going to look in introducing the AjaxAttributes.

All old Ajax-related methods like #getChannel(), #getPrecondition(), #getSuccessScript(), #getFailureScript(), ... now donate to the AjaxAttributes and are marked as deprecated. Still around for backward compatibility.
See LinksPage example to see how the old methods should be in 6.0.

Most of the wicket-examples work.
AjaxFallback*** are broken at the moment.
Pooling of XMLHttpRequest objects is not preserved because I didn't find the reason we do this in the old code.

www.json.org/java classes are used to generate the JSON but we can remove them if needed. Our needs are really simple.

TODO:

fix all wicket-examples

use channels

the old decorateScript(Component, CharSeq) is now just BeforeHandler, i.e. there is no way to add script at the end of the original. decorateSuccess/Failure can be used to execute something after the Ajax call. Do we need handler that is executed immediately after the start of the Ajax call?

Martin Grigorov
added a comment - 23/Nov/11 13:13 - edited A patch with AjaxAttributes.
This is the current state. It is not yet finished!
The idea is:
AjaxEventBehavior produces something like: Wicket.Ajax.do(attrs), where attrs is a JSON object like:
{
u: 'editable-label?6-1.IBehaviorListener.0-text1-label', // url
m: 'POST', // method name. Default: 'GET'
c: 'label7', // component id (String) or window for page
e: 'click', // event name
sh: [], // list of success handlers
fh: [], // list of failure handlers
pre: [], // list of preconditions. If empty set default : Wicket.$(settings
{c}
) !== null
ep: {}, // extra parameters
async: true|false, // asynchronous XHR or not
ch: 'someName|d', // AjaxChannel
i: 'indicatorId', // indicator component id
ad: true, // allow default
}
All old Ajax-related methods like #getChannel(), #getPrecondition(), #getSuccessScript(), #getFailureScript(), ... now donate to the AjaxAttributes and are marked as deprecated. Still around for backward compatibility.
See LinksPage example to see how the old methods should be in 6.0.
Most of the wicket-examples work.
AjaxFallback*** are broken at the moment.
Pooling of XMLHttpRequest objects is not preserved because I didn't find the reason we do this in the old code.
www.json.org/java classes are used to generate the JSON but we can remove them if needed. Our needs are really simple.
TODO:
fix all wicket-examples
use channels
the old decorateScript(Component, CharSeq) is now just BeforeHandler, i.e. there is no way to add script at the end of the original. decorateSuccess/Failure can be used to execute something after the Ajax call. Do we need handler that is executed immediately after the start of the Ajax call?
regenerate the unit tests' expectations
remove the old Ajax code in wicket-ajax.js

allowdefault - should we rename to stop-propagation which is a more commonly used term?

-------------------------------

one of the biggest pains right now is being able to pass extra parameters back to the server - as in i want to override ajax event behavior to append some parameters to the callback url. i think the easiest way to do this would be to add a callback parameters attribute that takes a list of functions to which the user can add and we evaluate in order, so:

u: 'editable-label?6-1.IBehaviorListener.0-text1-label',
cbp: [], <-- thats the default - no extra params, or leave it out to make json shorter

if we want to pass extra parameters i can say:

getattributes().addcallbackparameterfunction("function(p)

{ p.val=Wicket.$('#foo').value }");

which outputs

cbp: [function(p) { p.val=Wicket.$('#foo').value }

]

when we are building the callback url for a GET or parameter map for the POST we loop through these and let them build an extra map

-------------------------------

i think using JSON objects to get started is great. but, i think when we are done designing and we are optimizing we should use something of our own so that we dont include any attributes that dont have values. for example, if there is no indicator then the call to .do() should not have i:null, etc. this will make js as short as possible.

also, maybe we can add iheaderresponse.appendDoCall(attributes). this will allow us to further trim the js by removing multiple "Wicket.AJAX.do(" strings, and instead have one "Wicket.Ajax.do(" that takes a list of maps

Igor Vaynberg
added a comment - 23/Nov/11 16:49 looks good. a couple of comments:
allowdefault - should we rename to stop-propagation which is a more commonly used term?
-------------------------------
one of the biggest pains right now is being able to pass extra parameters back to the server - as in i want to override ajax event behavior to append some parameters to the callback url. i think the easiest way to do this would be to add a callback parameters attribute that takes a list of functions to which the user can add and we evaluate in order, so:
u: 'editable-label?6-1.IBehaviorListener.0-text1-label',
cbp: [], <-- thats the default - no extra params, or leave it out to make json shorter
if we want to pass extra parameters i can say:
getattributes().addcallbackparameterfunction("function(p)
{ p.val=Wicket.$('#foo').value }");
which outputs
cbp: [function(p) { p.val=Wicket.$('#foo').value }
]
when we are building the callback url for a GET or parameter map for the POST we loop through these and let them build an extra map
-------------------------------
i think using JSON objects to get started is great. but, i think when we are done designing and we are optimizing we should use something of our own so that we dont include any attributes that dont have values. for example, if there is no indicator then the call to .do() should not have i:null, etc. this will make js as short as possible.
also, maybe we can add iheaderresponse.appendDoCall(attributes). this will allow us to further trim the js by removing multiple "Wicket.AJAX.do(" strings, and instead have one "Wicket.Ajax.do(" that takes a list of maps
but these are all nice to haves in the future...

> allowdefault - should we rename to stop-propagation which is a more commonly used term?

A: preventDefault and stopPropagation are different thingies. Now I think allowDefault should be removed. Clicking on Ajax link or submitting a form with Ajax should not allow the default behavior (non-ajax click/submit). Probably I'll drop it.

> one of the biggest pains right now is being able to pass extra parameters back to the server - as in i want to override ajax event behavior to append some parameters to the callback url. i think the easiest way to do this would be to add a callback parameters attribute that takes a list of functions to which the user can add and we evaluate in order, so:

A: there is a way to add static parameters. I'll see how to make it easy to add dynamically calculated ones

-------------------------------

> i think using JSON objects to get started is great. but, i think when we are done designing and we are optimizing we should use something of our own so that we dont include any attributes that dont have values. for example, if there is no indicator then the call to .do() should not have i:null, etc. this will make js as short as possible.

A: the delivered JSON object contains only the entries with values.

> also, maybe we can add iheaderresponse.appendDoCall(attributes). this will allow us to further trim the js by removing multiple "Wicket.AJAX.do(" strings, and instead have one "Wicket.Ajax.do(" that takes a list of maps

A: this will be better solved with HeaderResponseDecorator that collects all onDomReady contributions and delivers them in one <script> element. This is part of the patch that Emond and Hielke work on.

Martin Grigorov
added a comment - 23/Nov/11 17:20 Not sure how to respond to a comment in Jira, so I copied it below...
> allowdefault - should we rename to stop-propagation which is a more commonly used term?
A: preventDefault and stopPropagation are different thingies. Now I think allowDefault should be removed. Clicking on Ajax link or submitting a form with Ajax should not allow the default behavior (non-ajax click/submit). Probably I'll drop it.
> one of the biggest pains right now is being able to pass extra parameters back to the server - as in i want to override ajax event behavior to append some parameters to the callback url. i think the easiest way to do this would be to add a callback parameters attribute that takes a list of functions to which the user can add and we evaluate in order, so:
A: there is a way to add static parameters. I'll see how to make it easy to add dynamically calculated ones
-------------------------------
> i think using JSON objects to get started is great. but, i think when we are done designing and we are optimizing we should use something of our own so that we dont include any attributes that dont have values. for example, if there is no indicator then the call to .do() should not have i:null, etc. this will make js as short as possible.
A: the delivered JSON object contains only the entries with values.
> also, maybe we can add iheaderresponse.appendDoCall(attributes). this will allow us to further trim the js by removing multiple "Wicket.AJAX.do(" strings, and instead have one "Wicket.Ajax.do(" that takes a list of maps
A: this will be better solved with HeaderResponseDecorator that collects all onDomReady contributions and delivers them in one <script> element. This is part of the patch that Emond and Hielke work on.

Sven Meier
added a comment - 29/Nov/11 07:25 Nice work.
Question:
Any reason why "e" and "f" are not members of AjaxRequestAttributes (e.g. no AjaxRequestAttributes#form), but added directly to the json object?

Because initially I thought they are not common, like the other attributes are. But now I see the event is also needed for the timer behaviors which don't extend AjaxEventBehavior and I'm thinking to move the in ARA.

My current problem is that jQuery Ajax doesn't support multipart form submit... And I don't want to use additional jQuery plugin for that, so I'm working on reusing our previous code for that. But now it doesn't look so nice anymore :-/

Martin Grigorov
added a comment - 29/Nov/11 08:07 Because initially I thought they are not common, like the other attributes are. But now I see the event is also needed for the timer behaviors which don't extend AjaxEventBehavior and I'm thinking to move the in ARA.
My current problem is that jQuery Ajax doesn't support multipart form submit... And I don't want to use additional jQuery plugin for that, so I'm working on reusing our previous code for that. But now it doesn't look so nice anymore :-/

some food for thought that maybe Wicket can incorporate into it's javascript layer (pardon if this is already handled)

"In some cases your scripts will load after the user has clicked on something that requires a JavaScript function to handle the click. It’s possible you have a pure HTML version, but if the user has JavaScript enabled then we want to use it, even if the JavaScript hasn’t loaded yet. There needs to be a way of handling events before all of the assets have finished loading."

Robert McGuinness
added a comment - 30/Nov/11 01:15 some food for thought that maybe Wicket can incorporate into it's javascript layer (pardon if this is already handled)
"In some cases your scripts will load after the user has clicked on something that requires a JavaScript function to handle the click. It’s possible you have a pure HTML version, but if the user has JavaScript enabled then we want to use it, even if the JavaScript hasn’t loaded yet. There needs to be a way of handling events before all of the assets have finished loading."
http://dailyjs.com/2011/11/28/flickr-async/

Martin Grigorov
added a comment - 30/Nov/11 23:17 Robert,
I'll be thankful for some help on this.
I also saw something similar described at https://plus.google.com/u/0/115060278409766341143/posts/ViaVbBMpSVG but I don't feel that I'm that deep in JavaScript

Martin Grigorov
added a comment - 19/Dec/11 14:36 - edited The patch is committed to current trunk.
There are few known problems:
ModalWindow example doesn't work completely (FIXED)
AjaxEditable**Label doesn't support the special behavior for ENTER/ESCAPE keys (FIXED)
the AjaxCheckBox in Ajax Todo List example doesn't mark itself as selected (FIXED)
KittenCaptcha example doesn't select the kittens (FIXED)
Ajax call throttling should be integrated somehow with the event handlers (FIXED)
These problems will be debugged soon.

It seems there are still some problems left. If you search the wicket code for Wicket.Ajax.post and Wicket.Ajax.get, you'll find a few calls to the wicket AJAX api that still use the old format, which no longer works. Some of them are in the deprecated methods in AbstractDefaultAjaxBehavior, but for example AjaxFormComponentUpdatingBehavior.getEventHandler also uses the old API (with a string). Perhaps these methods should simply be removed? Users who changed the generated AJAX scripts probably have no problems upgrading to the new API.

One more thing: AjaxEventBehavior.renderHead checks if the target is AJAX. That check is not needed anymore. The IHeaderResponse implementation for AJAX requests already renders onDomReady scripts as appended JS.

Emond Papegaaij
added a comment - 29/Dec/11 14:02 It seems there are still some problems left. If you search the wicket code for Wicket.Ajax.post and Wicket.Ajax.get, you'll find a few calls to the wicket AJAX api that still use the old format, which no longer works. Some of them are in the deprecated methods in AbstractDefaultAjaxBehavior, but for example AjaxFormComponentUpdatingBehavior.getEventHandler also uses the old API (with a string). Perhaps these methods should simply be removed? Users who changed the generated AJAX scripts probably have no problems upgrading to the new API.
One more thing: AjaxEventBehavior.renderHead checks if the target is AJAX. That check is not needed anymore. The IHeaderResponse implementation for AJAX requests already renders onDomReady scripts as appended JS.