i am +1 to drop it into beta4, Then if somehow we encounter many bugs because of that we could undo it But don't think this is really needed. Portlet support is also a nice marketing yell
johan On 9/18/07, Ate Douma <[EMAIL PROTECTED]> wrote: > > Guys, > > > I'd like to respond here on the concerns raised by Al, Gwyn and Kent on > the "Re: [jira] Resolved: (WICKET-647) New Wicket Portlet support" thread. > > As I created a dedicated JIRA issue, WICKET-983, for this as well as this > specific mail subject, I think we should try to centralize the discussion > about it here. > > First of all, thanks a lot for taking the time to look into this and > review the changes I propose for adding portlet-support. > > I do understand the concerns raised but I'd like to point out that, at > least in my view, the impact for *normal* wicket runtime (e.g. servlet, > not portlet) the > changes really are very, very minimal. > > Al, Gwyn and Kent: you indicate that you see major and/or fundamental > changes to wicket, but please make explicit which changes you are referring > to. > Now I only have something like "the portlet changes /do/ touch some > fundamental bits of Wicket", "making major changes to the codebase" and "It > seems that > those changes are quite fundamental" to respond to. > > Anyway, I'll try to make clear why in my view the portlet changes aren't > going to cause "major" or "fundamental" changes in the core wicket. > > If you evaluate the issues I listed in WICKET-983 and how I resolved > those, you'll see that the changes which *might* have a small to zero effect > on the > standard wicket behavior are the following: > > 1) WICKET-649: fix appending query parameters > a) replacing blindly appending "&" with a check if it really > should be a "?" > In my view harmless and actually really improving wicket quality > itself > > 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. > > d) IOnChangeListener components with > wantOnSelectionChangedNotifications()==true > If you review my changes you'll find out there are *zero* changes when > running in a servlet environment, as is with almost all other changes. > Based on the boolean evaluation of RequestContext.isPortletRequest(), > which is always false in a servlet environment as you can imagine, only the > original > code is executed. Only the portlet specific behavior is "wrapped" in a > simple if (RequestContext.isPortletRequest())... nothing more. > > 2) WICKET-650: namespacing component markupId > This change does nothing when not running in a portlet environment > > 3) WICKET-651: extending IHeaderResponse > This change only adds a close() and isClosed() method to IHeaderResponse > and delegates getResponse() to a getRealResponse() implementation. > These changes also have no side-effect, all current behavior is > maintained. It just allows capturing the HeaderResponse when running in a > portlet context. > It also allows you now to do the same in a servlet environment if you > would want that, so it is an expansion of the api but it is not used other > than from > portlet specific code. > > 4) WICKET-657: upgrading wicket-examples to require servlet api 2.4 > I don't consider this a "major" change. For one, its just an example and > not touching wicket core at all. Furthermore, I doubt there will be many > users still > stuck on a servlet 2.3 container, especially as wicket-examples already > requires Java 1.5 anyway. > Anyway, this change isn't critical for merging the portlet support in the > trunk but for demonstration purposes it would be nice to be able to use it. > > 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. > > 6) WICKET-926: recognizing popup/detached pages urlFor calls > This change also does nothing when not running in a portlet environment. > But I did listed in WICKET-983 because the solution I chose didn't exactly > result in > "beautiful" code... I couldn't come up with something else though which > would be just as transparent and without *any* side-effects. > So, I'm actually hoping someone calls me stupid and comes up with a more > "feel good" solution :) > > That's it. All other changes to existing wicket classes will only come > into effect when run in a portlet environment. When running in a servlet > environment > Wicket behaves exactly as before! > > Concluding: in my view the only real "issue" we might want to discuss is > the Ajax.Request.get(path) -> .post(body) change. > I personally don't see it as an issue, but if that change would be viewed > as unacceptable/dangerous for the current trunk, it should not be difficult > to come up > with another solution which will keep the current behavior in a servlet > environment (although that then will have to be a little bit less > transparent I'm afraid). > > So, if this assessment is valid, I really don't see why portlet support > couldn't be integrated into the 1.3.0 trunk. > Again, I do understand the concerns raised, but don't forget quite a few > major changes (like for bugfixes, but probably not even only for that) have > been > committed, and still are committed to the trunk too. Some of those might > have (much) more side-effects and consequences then adding > portlet-support... > > Delaying portlet-support to after the 1.3 release really would be a shame > in this light in my view. > For those of us who don't need/care about portlet it won't matter, but I > know quite a few developers and project teams are eager to be able to use > Wicket for > portlet development. Waiting until the 1.4 branch stabilizes really means > cutting them out. > Causing a delay of the 1.3 release isn't good for the community (and I > don't think should be needed) but delaying portlet-support until after the > 1.3 release > isn't good for the community either. > > I do hope my above explanations make it a bit clearer what the real issues > are to discuss. > If anyone of you still have reservations and/or major objections of > merging portlet-support in the current trunk, please give it head and toe > and indicate which > changes you think are questionable so we can properly discuss them. > > Regards, > > Ate > > Ate Douma wrote: > > I've created https://issues.apache.org/jira/browse/WICKET-983 to help > > out reviewing the portlet-support changes. > > > > Although I initially planned to create separate "sub" patches for > > reviewing the important wicket changes individually, this turned out to > > be a > > rather impossible task. The complete size of the full patch is almost > > 200k and several different issues I needed to solve concerned the same > > classes/files. Breaking all that up in digestible peaces would cost just > > to much time to be realistic. > > > > I did however upload a full patch file and provide a short summary of > > the most important changes with links to the specific subtask issues of > > WICKET-647. > > These subtasks provide more detailed descriptions of the issues at hand > > as well as the commit logs of the changes I made. > > > > All committers or otherwise interested: please take some time to at > > least read the description of the problems I needed to solve. > > If you see anything you don't like or would have chosen a different > > solution, please bring it up here so we can discuss it. > > > > I would like to merge the portlet support branch into the trunk before > > the -beta4 cutoff, so your input on this is very much appreciated. > > > > Thanks, > > > > Ate Douma > > > > > > > >
