[ 
https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003211#comment-13003211
 ] 

Maarten Billemont commented on WICKET-3218:
-------------------------------------------

Firstly, I apologize for the strong language.  Upon re-reading my own comment, 
I should've left out certain nonfactual keywords and worded things a bit 
differently.

I agree that breakage at runtime is not ideal.  OTOH, breakage when migrating 
to 1.5 is inevitable and completely expected by all developers.  Additionally, 
I'm sure there will be plenty of other areas where migration issues will only 
surface at runtime.  In this particular case, the issue will become clear as 
soon as the application is launched for the first time and the fix is trivial, 
unless developers are doing other nasty stuff that should really be fixed 
whether or not it causes immediate bugs or only rare side-effects.  That, IMO, 
excuses the exception I propose as acceptable.

Might I also indicate that the current resolution is also not backwards 
compatible and forces everyone that currently consistently uses onInitialize in 
their pages as well as in their other components, to move their perfectly good 
code into a different structure.  Why not force this change on another set of 
people, where it will actually do more good?  Just because the other group is 
larger?  I think, for the sake of the future health of your framework, it's 
important that you can make unpopular decisions that benefit a clean and 
consistent API.

> Component#onInitialize is broken for Pages
> ------------------------------------------
>
>                 Key: WICKET-3218
>                 URL: https://issues.apache.org/jira/browse/WICKET-3218
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket-core
>    Affects Versions: 1.4.14
>            Reporter: Carl-Eric Menzel
>            Assignee: Igor Vaynberg
>             Fix For: 1.5-RC1
>
>         Attachments: 
> 0001-delay-oninitialize-until-just-before-onconfigure.patch
>
>
> As first mentioned at 
> http://mail-archives.apache.org/mod_mbox/wicket-dev/201012.mbox/%3c1291238699.1749.1408166...@webmail.messagingengine.com%3E
>  , I think the current (Wicket 1.4.14) implementation of 
> Component#onInitialize is broken for Pages. Pages get initialized as soon as 
> the first component is added, which is correct. But this usually happens 
> within the constructor of the page, which means that the page object isn't 
> fully initialized yet. The entire point of having onInitialize, however, is 
> to be able to do further work once all constructors have run. See 
> https://github.com/duesenklipper/wicket-oninitialize for a quickstart that 
> demonstrates the problem.
> Pedro Santos suggested in the above thread to just switch the entire object 
> construction to onInitialize. I don't think this is a good idea, because
> 1) it is completely counter-intuitive
> 2) it is not always realistic to have an entire class hierarchy not using the 
> constructor just because a subclass somewhere might want to use onInitialize
> 3) it is inconsistent with onInitialize behavior for all other (non-Page) 
> components. Here I can easily mix work in the constructor with onInitialize.
> I propose the following patch:
> - override onInitialize in Page and make it final, so Pages can't use this 
> any more. This should not cause any unnecessary breaking, since currently 
> it's not working for pages anyway.
> - introduce Page#onPageInitialize to provide a safe alternative to 
> onInitialize
> - make a special case for Page in Component's beforeRender to fire 
> Page#onPageInitialize if necessary
> Yes, this is a bit of special casing for Page, but there's quite a lot of 
> that needed for Page anyway. I think the impact of this should be minimal.
> My page includes documentation and a new testcase that verifies the new 
> behavior. I modified the old ComponentInitializationTest to reflect the fact 
> that Page doesn't get onInitialize any more.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to