Gwyn Evans wrote:
On Tuesday, September 18, 2007, 1:14:11 PM, Ate <[EMAIL PROTECTED]> wrote:
b) moving AbstractAjaxBehavior.getCallbackUrl() appending
"wicket:ignoreIfNotActive=true" to WebRequestCodingStrategy.encode().
This is a quite simple reordering of internal processing with
exactly the same functionality and result as outcome, quite harmless I think
c) wicket-ajax Wicket.Ajax.Request.get(path) checking on query
parameters and in that case replacing it with a Request.post(body) call
This one I think can/should be discussed as it really does
result in a *technically* different behavior.
I do think in the case of Ajax requests, this really doesn't
matter as it won't effect the browser state at all, and my solution really is
most transparent one I could think of.
But, as it also changes the behavior in a plain servlet
environment, and as such it I could understand people would feel a bit
uncomfortable with it.
I already suggest in the description for this change I'm open
to other solutions, like only doing this when running in a portlet environment.
Then, this issue would become moot for the servlet environment,
we just need to find some way to indicate the current environment
(servlet|portlet)
in javascript to the browser.
5) WICKET-924: non-relative urls in Ajax.Request.redirect callback handling
Actually, I would consider proposing this change regardless of the
discussion of portlet-support. The current hard coded expectancy an Ajax
callback redirect
can and may only be a Wicket relative url doesn't seem very
flexible. For the portlet-support this change is needed though.
And, if the redirect url *is* relative as expected in the current
implementation, my changes won't do anything anyway.
But if there are major objections to it, we could discuss solving
it in a similar way as we could do with the Ajax.Request.get(path) ->
.post(body) change.
If we can push the current runtime environment (servlet|portlet) to
the browser the ajax script could maintain the current behavior when running in
a servlet.
I think the areas that I'm concerned about are primarily these ones,
not with respect to the specific changes themselves, but more of a
concern as to whether we should be changing the trunk right now & how
much of an impact these would have on non-portlet specific codepaths.
Gwyn,
The only thing I can say about this is: please do review the actual changes I
propose and see for yourself the amount of impact they have on non-portlet
codepaths.
As I've stated before, these changes are nothing more than quite simple "bug" fixes from a portlet perspective, and I really, really did my best to make them
cause no side-effects in a non-portlet environment.
Since -beta3, there have been several other commits to the trunk which also
affect critical areas like url handling: should those not have been applied
either?
I'm not questioning those to be sure, but please do (re)view my proposed
changes in this perspective.
Of the proposed changes you highlighted above, I think only the Ajax.Request.get(path) -> Ajax.Request.post(body) change might be considered to potentially have
a side-effect yet unknown. I don't expect it, but of course there's always a chance.
But the same can be said for many other changes/bugfixes committed since -beta3
and that doesn't hold us back either, does it?
We review them and if need be question them, but in general accept them if it
does improve Wicket and we don't see a danger of it breaking things.
I won't (nor can) say my changes are 100% perfect and guaranteed not going to
break anything, but then: who can?
I'm just asking for you and the others to review my changes in the same light, and if you actually do see danger in it breaking stuff, then I'm all ears and
definitely willing to try to fix it. And if needed even pull back my proposal if I can't.
But without concrete information where you or the others do see potential problems, it is difficult for me to answer the type of concerns you raise, especially
as I don't see any of those problems myself yet.
I guess the specific concern relates to whether we have sufficient
confidence, whether or not that's via unit test, to be happy with the
changes.
I think that really is the important question here.
And I don't know what else I can do or say right now to provide you and the others more confidence other then just asking for a proper review of my changes and
testing it out.
The amount of changes to the existing code isn't very big and won't cost you
much time to review.
If you're willing to do so, I'd suggest checking out the -beta3 release and apply the patch file I attached to WICKET-983. Then, using the Eclipse Team
Synchronization Perspective, you can get a quick view on all the changes, within context.
I also provided a build wicket-examples.war based on the portlet-support branch
for testing purposes which you can deploy in any servlet container:
http://people.apache.org/~ate/wicket/wicket-examples.war
HTH,
Ate
At the moment I'd have a "-0.2" position on things, i.e. slightly
negative, as I want to get a 1.3 release out and have concerns that
this might delay things, but open to persuasion & would welcome some
input from someone more familiar with the core than I am who might be
able to comment as to if my concerns are likely to be unfounded.
/Gwyn
