You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Carl-Eric Menzel (JIRA)" <ji...@apache.org> on 2010/12/02 18:57:11 UTC
[jira] Created: (WICKET-3218) Component#onInitialize is broken for
Pages
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
Affects Versions: 1.4.14
Reporter: Carl-Eric Menzel
Attachments: 0001-added-page-onpageinitialize.patch
As first mentioned at http://mail-archives.apache.org/mod_mbox/wicket-dev/201012.mbox/%3C1291238699.1749.1408166705@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.
-
You can reply to this email to add a comment to the issue online.
[jira] [Commented] (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13011574#comment-13011574 ]
Igor Vaynberg commented on WICKET-3218:
---------------------------------------
initial cascade is now delayed just prior to first render of the page. however, once the page has been initialized further component initialization will happen on immediate availability of the page just like it does now.
> 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-RC3
>
> 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.1408166705@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
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12966393#action_12966393 ]
Igor Vaynberg commented on WICKET-3218:
---------------------------------------
removed patch - no good - parent should be initialized before child - patch broke that for pages.
> 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
> Affects Versions: 1.4.14
> Reporter: Carl-Eric Menzel
>
> As first mentioned at http://mail-archives.apache.org/mod_mbox/wicket-dev/201012.mbox/%3C1291238699.1749.1408166705@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.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (WICKET-3218) Component#onInitialize
is broken for Pages
Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003032#comment-13003032 ]
Maarten Billemont edited comment on WICKET-3218 at 3/5/11 7:56 PM:
-------------------------------------------------------------------
The solution is really simple. Stop with the inconsistencies, stop with allowing constructor-created components everywhere, always. That's the whole reason onInitialize was introduced.
This issue deals with the fact that things go wrong when onInitialize isn't used properly. And instead of making your users do the right thing, you break your API consistency, ludicrous.
put an assert in add() that forbids its usage during construction. You never want people adding components to their page during construction. Ever. Teach your userbase how to do it right, and make it part of the "upgrade-to-1.5" learning curve.
Pages should behave the same way as any component as far as component hierarchy construction is concerned, there's absolutely no sense to confusing your users and breaking that consistency.
Re: your example:
EditUserPage page=new EditUserPage(user); page.switchToAddressEditMode();
switchToAddressEditMode() shouldn't be doing anything at all with components. None of your methods should with the exception of rarely replacing a component in a wicket on* method. Instead, everything that you want to do with regards to modifying how and what your component renders should preferably be toggled with models or state that gets checked in onConfigure.
If you want to protect your users from themselves and not sacrifice your API to suit the idiots, then tell them they're doing it wrong with a runtime exception.
was (Author: lhunath):
The solution is really simple. Stop with the inconsistencies, stop with allowing constructor-created components everywhere, always. That's the whole reason onInitialize was introduced.
This issue deals with the fact that things go wrong when onInitialize isn't used properly. And instead of making your users do the right thing, you break your API consistency, ludicrous.
put an assert in add() that forbids its usage during construction. You never want people adding components to their page during construction. Ever. Teach your userbase how to do it right, and make it part of the "upgrade-to-1.5" learning curve.
Pages should behave the same way as any component as far as component hierarchy construction is concerned, there's absolutely no sense to confusing your users and breaking that consistency.
Re: your example:
EditUserPage page=new EditUserPage(user); page.switchToAddressEditMode();
switchToAddressEditMode() shouldn't be doing anything at all with components. None of your methods should. Your component hierarchy should be constructed ONCE in onInitialize, and remain STATIC for the rest of your component's lifecycle. Everything that you want to do with regards to modifying how and what your component renders should be toggled with models or state that gets checked in onConfigure (with the rare exception of sometimes replacing a component in onConfigure/onBeforeRender).
If you want to protect your users from themselves and not sacrifice your API to suit the idiots, then tell them they're doing it wrong with a runtime exception.
> 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.1408166705@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
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Carl-Eric Menzel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12966514#action_12966514 ]
Carl-Eric Menzel commented on WICKET-3218:
------------------------------------------
Ok, you are right about that. I missed that. After your comment on the dev list that the whole initialization thing could be moved back to before configure(), I started on a different patch. The main concern for me is still that I need to be able to have an initialize step that is guaranteed to run after the constructors.
> 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
> Affects Versions: 1.4.14
> Reporter: Carl-Eric Menzel
>
> As first mentioned at http://mail-archives.apache.org/mod_mbox/wicket-dev/201012.mbox/%3C1291238699.1749.1408166705@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.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WICKET-3218) Component#onInitialize is broken for
Pages
Posted by "Carl-Eric Menzel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl-Eric Menzel updated WICKET-3218:
-------------------------------------
Attachment: 0001-delay-oninitialize-until-just-before-onconfigure.patch
A new attempt at resolving this.
I moved the call to initialize() into internalBeforeRender(), since unlike configure() this can't be triggered by framework users.
Components that are added before the Page itself has been initialized now are now simply initialized with the cascading fireInitialize() when the Page gets its initialize call. Components that are added after that will be initialized by Page#componentAdded.
This way the initialization order is guaranteed to follow the component hierarchy.
IMO, this restores a consistent and intuitive behavior for onInitialize for *all* components including Pages, without breaking any existing code.
I adjusted ComponentInitializationTest to always use tester.startPage, since initialization is no longer triggered by add(). The tests that still made sense in this light still work. testPropagation was removed, since there is no immediate propagation anymore, and eventual propagation down the component tree is included in testInitializationOrder.
One thing I'm not so happy about: I needed to distinguish between "already initialized" and "currently initializing" to delay initialization for components that are added in an onInitialize method. To do that I needed a new flag FLAG_INITIALIZING. Unfortunately the int space for flags was exhausted, so I had to turn Component#flags into a long.
The cost of this is 4 extra bytes. I'm not sure whether that is acceptable. If not, it could probably be replaced with a single extra boolean in Component, which would reduce the cost but not eliminate it.
> 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
> Affects Versions: 1.4.14
> Reporter: Carl-Eric Menzel
> 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.1408166705@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.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WICKET-3218) Component#onInitialize is broken for
Pages
Posted by "Carl-Eric Menzel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl-Eric Menzel updated WICKET-3218:
-------------------------------------
Attachment: 0001-added-page-onpageinitialize.patch
A patch against Wicket 1.4.14 that does what I outlined in the bug description. Includes appropriate test cases.
> 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
> Affects Versions: 1.4.14
> Reporter: Carl-Eric Menzel
> Attachments: 0001-added-page-onpageinitialize.patch
>
>
> As first mentioned at http://mail-archives.apache.org/mod_mbox/wicket-dev/201012.mbox/%3C1291238699.1749.1408166705@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.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (WICKET-3218) Component#onInitialize
is broken for Pages
Posted by "Chris Colman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003750#comment-13003750 ]
Chris Colman edited comment on WICKET-3218 at 3/8/11 3:34 AM:
--------------------------------------------------------------
+1 for onInitialize being overridable in pages. A 'post constructor' initialize method is available in just about every GUI framework I've ever worked on OWL, MFC, .net, AWT, Swing, Echo. Wicket is the first one I've ever used that encourages the dangerous practice of adding child components while the parent is only 'partially constructed' (ie. in a super class constructor).
I discussed this back in the old days ('07) in the context of a problem with getVariation() that was overridden and hence caused problems when being called during construction. The specific case of getVariation() was fixed but I still feel that the onInitialize is a very good idea. With sufficient documentation it won't be a problem for the newbies and anyone whose ever used any other GUI framework will embrace it.
http://apache-wicket.1842946.n4.nabble.com/Lifecycle-issue-with-getVariation-td1928017.html
was (Author: chrisc):
+1 for onInitialize being overridable in pages. A 'post constructor' initialize method is available in just about every GUI framework I've ever worked on OWL, MFC, .net, AWT, Swing, Echo. Wicket is the first one I've ever used that encourages the dangerous practice of adding child components while the parent is only 'partially constructed' (ie. in a super class constructor).
I discussed this back in the old days ('07) in the context of a problem with getVariation() that was overridden and hence called problems when being called during constructor. The specific case of getVariation() was fixed but I still feel that the onInitialize is a very good idea. With sufficient documentation it won't be a problem for the newbies.
http://apache-wicket.1842946.n4.nabble.com/Lifecycle-issue-with-getVariation-td1928017.html
> 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.1408166705@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
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Chris Colman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003750#comment-13003750 ]
Chris Colman commented on WICKET-3218:
--------------------------------------
+1 for onInitialize being overridable in pages. A 'post constructor' initialize method is available in just about every GUI framework I've ever worked on OWL, MFC, .net, AWT, Swing, Echo. Wicket is the first one I've ever used that encourages the dangerous practice of adding child components while the parent is only 'partially constructed' (ie. in a super class constructor).
I discussed this back in the old days ('07) in the context of a problem with getVariation() that was overridden and hence called problems when being called during constructor. The specific case of getVariation() was fixed but I still feel that the onInitialize is a very good idea. With sufficient documentation it won't be a problem for the newbies.
http://apache-wicket.1842946.n4.nabble.com/Lifecycle-issue-with-getVariation-td1928017.html
> 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.1408166705@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
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003219#comment-13003219 ]
Igor Vaynberg commented on WICKET-3218:
---------------------------------------
currently people can migrate their apps from 1.4 to 1.5 in about a day or two. but this change would take many days as every single page and component would have to be touched. further complications come from the fact that users will have to wait for all the non-core libraries used in their applications to be upgraded as well since this is a world-breaking change. i think such change is better suited for wicket 2.0 instead of a minor version release...but like i said, start a vote/discussion thread to get the community feedback.
> 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.1408166705@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
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003032#comment-13003032 ]
Maarten Billemont commented on WICKET-3218:
-------------------------------------------
The solution is really simple. Stop with the inconsistencies, stop with allowing constructor-created components everywhere, always. That's the whole reason onInitialize was introduced.
This issue deals with the fact that things go wrong when onInitialize isn't used properly. And instead of making your users do the right thing, you break your API consistency, ludicrous.
put an assert in add() that forbids its usage during construction. You never want people adding components to their page during construction. Ever. Teach your userbase how to do it right, and make it part of the "upgrade-to-1.5" learning curve.
Pages should behave the same way as any component as far as component hierarchy construction is concerned, there's absolutely no sense to confusing your users and breaking that consistency.
Re: your example:
EditUserPage page=new EditUserPage(user); page.switchToAddressEditMode();
switchToAddressEditMode() shouldn't be doing anything at all with components. None of your methods should. Your component hierarchy should be constructed ONCE in onInitialize, and remain STATIC for the rest of your component's lifecycle. Everything that you want to do with regards to modifying how and what your component renders should be toggled with models or state that gets checked in onConfigure.
If you want to protect your users from themselves and not sacrifice your API to suit the idiots, then tell them they're doing it wrong with a runtime exception.
> 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.1408166705@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
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
[ 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.1408166705@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
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Szádeczky-Kardoss Szabolcs (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12987077#action_12987077 ]
Szádeczky-Kardoss Szabolcs commented on WICKET-3218:
----------------------------------------------------
Hi Igor and Carl,
I think this issue is really a misunderstanding of the onInitialize concept, as it was a really needed feature. This was the reason I switched to 1.5-M3, and then BANG! in RC1 this is broken. Let me explain how to use it in the right way, and then my use case for which this is much needed.
USAGE
As you have pointed out, onInitialize gets called at the first time when a component gets added to the Page, or - if this doesn't happen in the constructor then - in the render phase (internalPrepareForRender method) at latest. Well, if you add any component to the page in the constructor then onInitialize is really not of much use, actually in this case it's better not to use it all. However if you start making pages as I do, then it becomes a joy to work with.
The solution is simple: Don't add any component to the Page in the constructor, use onInitialize for that purpose. If you advocate this as a best practice then the use cases stated below will be much easier to make than without the onInitialize (and using only constructor).
The constructor is anyway best suited only for setting up models, performing (some or all of the) service calls and other things needed to ensure that the given Page is the one to show to the user, without going into costly component additions to the Page.
USE CASES
- In my current project we have a common base page from which all other pages subclasses and so we share a common layout, some common panels and common functionality in all pages. However once in a while it might be needed to hide or replace one of the common panels in only one page but leave it as common in all of the others. If you only could do this in the constructor then that will be a really pain. The reason in short is that irrespective of whether you use overriden "panel adder methods" or any other solution you are still in object construction phase and that puts quite a few restrictions on variable initialization order. I know, I did it, and there are only hacking workarounds for this. On the other hand, onInitialize is a super elegant way to use overriden methods or any cool OO technique since you are not constrained any more by the "Construction phase".
- An other use case is that the user is doing stuff on any page, it can be anything. Irrespective of what he/she is doing, something is happening in the background, perhaps by an other user's action. The next time the first user is refreshing this page or going to another page, I want the user to be notified, in a common way, in a common code. This can be also achieved without onInitialize with more or less hacking (especially when we want the user notified when staying on the same page), but with onInitialize this is a much cleaner. The reason is that prepareForRender (which by the way also got final, why?) precedes onInitialize and so it is possible to do this check there.
FINAL/NON-FINAL :) THOUGHTS
Sorry for getting so long, my only point is that onInitialize (and prepareForRender) not being final was one of the great achievements of 1.5 (backported into 1.4, or was it the other way around?). There were really use cases for this, especially the avoidance of the Java Object Construction Phase limitations (and the possibility of not having ugly big constructors :)). Sure this can be misused or used wrongly as a lot of other things in Wicket, but this is not a reason to limit the good usage of this. You can put it in the javadoc, that only use onInitialize when you don't do add operations in the constructor. Actually I would also ask what's the point of onInitialize anywhere else than for Pages? At least I could live without it anywhere else, but not for Pages.
I hope you will change both methods back to non-final. If not, then I will have to revert to 1.4 and possibly never use 1.5, which would be a sad thing.
> 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
> 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.1408166705@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.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003194#comment-13003194 ]
Igor Vaynberg commented on WICKET-3218:
---------------------------------------
@Maarten, a change like that will break every single application and library written so far. Worse, it will break it with a runtime exception. you make a lot of assumptions in your message which core committers would not agree with, you have the luxury of viewing this from a single application's point of view - we do not.
But, even if we do not agree with you we always listen to the community. Go ahead and start a vote on the user list to introduce this change, and explain how applications and libraries would have to be migrated. Dont forget to explain how people should carry over constructor parameters in fields, etc, so child components that have easy access to them now can access them once they are created in onInitialize().
If it doesnt fly you can still do what you want, as you have yourself clearly explained, by putting that code in onConfigure().
> 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.1408166705@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
[jira] [Reopened] (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg reopened WICKET-3218:
-----------------------------------
> 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-RC3
>
> 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.1408166705@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
[jira] Updated: (WICKET-3218) Component#onInitialize is broken for
Pages
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg updated WICKET-3218:
----------------------------------
Attachment: (was: 0001-added-page-onpageinitialize.patch)
> 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
> Affects Versions: 1.4.14
> Reporter: Carl-Eric Menzel
>
> As first mentioned at http://mail-archives.apache.org/mod_mbox/wicket-dev/201012.mbox/%3C1291238699.1749.1408166705@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.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Szádeczky-Kardoss Szabolcs (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12987194#action_12987194 ]
Szádeczky-Kardoss Szabolcs commented on WICKET-3218:
----------------------------------------------------
Thanks for the quick reply. The point is yours with my second use case, that's fine with onConfigure. However I have still a problem with onInitialize. If it remains final as in RC1, then for Pages there's no other way to do initialization than in the constructor, right? All other Components can have this nice feature, but not Pages. Why? If you really want to protect users, then a much nicer solution would have been NOT to initialize Pages when components get added, but only before the render phase. I understand that most situations could be solved with a panel replace maybe in onConfigure, but IMHO this is kind of a hack compared to a nice OO solution that actually are very widespread in Wicket elsewhere. And what makes Wicket lightyears ahead of JSP, JSF and Co.
If you still insist on the current solution, would it be not possible to add an extra initialize method for Pages that gets called guaranteed only once, and guaranteed not while the constructor is running (probably before the render phase like now)? For me this would be essential, and I can hopefully show another use case of mine for this purpose:
On some pages there might be an important form working with session-bound data that we want to protect even when the user navigates away via some link other than submit or cancel. Imagine a shopping cart page where the user can set the quantities but if he abruptly clicks on a featured product in the sidebar I want to store all the quantity changes he has made in the cart. A very efficient solution can be done with AjaxFormSubmitBehavior, that I add to each link on each panel on such Pages. However each Panel must be made aware of this important form at creation or initialization time. Since my panels get added in my common base Page, without onInitialize the only nice way to pass this form to it is through the super constructor call, however the form is definitely not available in the first line of the constructor where super(...) is. Of course workarounds can be made, as I have made one myself that included an extra Page-managed initialized attribute and some extra glues, but onInitialize was really a super nice solution to this problem. I am open to other suggestions, but probably not leaving Pages as orphans that don't have a chance for nice initialization outside of a Java Constructor would make Wicket more concistent.
> 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
> 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.1408166705@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.
-
You can reply to this email to add a comment to the issue online.
[jira] [Commented] (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13010038#comment-13010038 ]
Maarten Billemont commented on WICKET-3218:
-------------------------------------------
I think the result of the community call for input on this issue was:
- Don't throw exception on add() from constructors,
- Don't make onInitialize() final,
- Don't run onInitialize() on componentAdded in Pages
Shall we resolve the issue this way?
> 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.1408166705@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
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12987080#action_12987080 ]
Igor Vaynberg commented on WICKET-3218:
---------------------------------------
the problem with leaving onInitialize() open in pages is that its way too easy to make the mistake and add something in the constructor and have really wonky unpredictable initialization order results like you mentioned. almost all our users are used to adding things in the constructor and unfortunately we have to protect our users from themselves.
prepareForRender() is final because it should not be used, there is onBeforeRender() which should be used as a callback.
as far as oninitialize() in non-pages, this is extremely useful. sounds like your app is built mostly with pages. a lot of the apps i work on have very dynamic UIs and are built almost exclusively with panels and panel swapping. in these cases panels act as pages.
your usecases can be achieved like this:
1) in onconfigure() do a simple check: if (get("panel1") instanceof baepanel) { replace(new specializedpanel("panel1"));}
2) you can do the same thing in onConfigure() or onBeforeRender()
> 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
> 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.1408166705@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.
-
You can reply to this email to add a comment to the issue online.
[jira] [Resolved] (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg resolved WICKET-3218.
-----------------------------------
Resolution: Fixed
Fix Version/s: (was: 1.5-RC1)
1.5-RC3
> 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-RC3
>
> 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.1408166705@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
[jira] Issue Comment Edited: (WICKET-3218) Component#onInitialize
is broken for Pages
Posted by "Maarten Billemont (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003032#comment-13003032 ]
Maarten Billemont edited comment on WICKET-3218 at 3/5/11 7:47 PM:
-------------------------------------------------------------------
The solution is really simple. Stop with the inconsistencies, stop with allowing constructor-created components everywhere, always. That's the whole reason onInitialize was introduced.
This issue deals with the fact that things go wrong when onInitialize isn't used properly. And instead of making your users do the right thing, you break your API consistency, ludicrous.
put an assert in add() that forbids its usage during construction. You never want people adding components to their page during construction. Ever. Teach your userbase how to do it right, and make it part of the "upgrade-to-1.5" learning curve.
Pages should behave the same way as any component as far as component hierarchy construction is concerned, there's absolutely no sense to confusing your users and breaking that consistency.
Re: your example:
EditUserPage page=new EditUserPage(user); page.switchToAddressEditMode();
switchToAddressEditMode() shouldn't be doing anything at all with components. None of your methods should. Your component hierarchy should be constructed ONCE in onInitialize, and remain STATIC for the rest of your component's lifecycle. Everything that you want to do with regards to modifying how and what your component renders should be toggled with models or state that gets checked in onConfigure (with the rare exception of sometimes replacing a component in onConfigure/onBeforeRender).
If you want to protect your users from themselves and not sacrifice your API to suit the idiots, then tell them they're doing it wrong with a runtime exception.
was (Author: lhunath):
The solution is really simple. Stop with the inconsistencies, stop with allowing constructor-created components everywhere, always. That's the whole reason onInitialize was introduced.
This issue deals with the fact that things go wrong when onInitialize isn't used properly. And instead of making your users do the right thing, you break your API consistency, ludicrous.
put an assert in add() that forbids its usage during construction. You never want people adding components to their page during construction. Ever. Teach your userbase how to do it right, and make it part of the "upgrade-to-1.5" learning curve.
Pages should behave the same way as any component as far as component hierarchy construction is concerned, there's absolutely no sense to confusing your users and breaking that consistency.
Re: your example:
EditUserPage page=new EditUserPage(user); page.switchToAddressEditMode();
switchToAddressEditMode() shouldn't be doing anything at all with components. None of your methods should. Your component hierarchy should be constructed ONCE in onInitialize, and remain STATIC for the rest of your component's lifecycle. Everything that you want to do with regards to modifying how and what your component renders should be toggled with models or state that gets checked in onConfigure.
If you want to protect your users from themselves and not sacrifice your API to suit the idiots, then tell them they're doing it wrong with a runtime exception.
> 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.1408166705@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
[jira] Commented: (WICKET-3218) Component#onInitialize is broken
for Pages
Posted by "selckin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003031#comment-13003031 ]
selckin commented on WICKET-3218:
---------------------------------
Would just like to say, i would really like it if onInitialize would keep being available in pages.
> 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.1408166705@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
[jira] Resolved: (WICKET-3218) Component#onInitialize is broken for
Pages
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg resolved WICKET-3218.
-----------------------------------
Resolution: Fixed
Fix Version/s: 1.5-M4
Assignee: Igor Vaynberg
fixed in 1.5. leaving in 1.4.x because making it final can break backwards compat.
i made the decision to make the method final in page. i think this makes more sense then delaying the call because by the time the page's constructor is finished the page should be initialized, eg code like this is commong
EditUserPage page=new EditUserPage(user); page.switchToAddressEditMode();
switchToAddressEditMode() may depend on certain components being added to the hierarchy and if they are added in oninitialize() which is delayed the method may fail.
thus, the principle of least surprise dictates we make it final in page.
> 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
> Affects Versions: 1.4.14
> Reporter: Carl-Eric Menzel
> Assignee: Igor Vaynberg
> Fix For: 1.5-M4
>
> 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.1408166705@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.
-
You can reply to this email to add a comment to the issue online.