You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Martin Makundi (JIRA)" <ji...@apache.org> on 2010/11/13 13:10:14 UTC
[jira] Created: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
------------------------------------------------------------------------------------------
Key: WICKET-3166
URL: https://issues.apache.org/jira/browse/WICKET-3166
Project: Wicket
Issue Type: Bug
Affects Versions: 1.4.13
Reporter: Martin Makundi
Attachments: Wicket-Quickstart.zip
Hi!
See attached quickstart with junit test reproducing the bug. See also patch proposal.
I have a page with two panels:
page.form.add(panel1);
page.form.add(panel2);
in some situations panel1 is not visible.
However, a form submit event will visit all formcomponents of panel1 via:
at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
This results in a crash because panel1 components are not prepared to
be invoked via isvisible when the panel itself is not visible.
I wonder if the component.isVisibleInHierarchy could be changed as
follows, to first check parent visibility:
public final boolean isVisibleInHierarchy()
{
Component component = this;
while (component != null)
{
Component componentParent = component.getParent();
if (((componentParent == null) ||
componentParent.isVisibleInHierarchy()) &&
component.determineVisibility())
{
component = componentParent;
}
else
{
return false;
}
}
return true;
}
Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12931959#action_12931959 ]
Hudson commented on WICKET-3166:
--------------------------------
Integrated in Apache Wicket 1.5.x #517 (See [https://hudson.apache.org/hudson/job/Apache%20Wicket%201.5.x/517/])
meh. this caching will probably slow things down. and it can be difficult to find all the places in code where caching should be invalidated. lets hold off until it is a hotspot.
Issue: WICKET-3166
caching for isVisibleInHieararchy
Issue: WICKET-3166
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Martin Makundi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Martin Makundi updated WICKET-3166:
-----------------------------------
Attachment: diff.txt
Fix patch proposal, few lines at Component.isVisibleInHierarchy only.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Martin Makundi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12931880#action_12931880 ]
Martin Makundi commented on WICKET-3166:
----------------------------------------
New fix proposal:
public final boolean isVisibleInHierarchy()
{
Component componentParent = getParent();
return ((componentParent == null) || componentParent.isVisibleInHierarchy()) && determineVisibility();
}
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12931803#action_12931803 ]
Hudson commented on WICKET-3166:
--------------------------------
Integrated in Apache Wicket 1.5.x #511 (See [https://hudson.apache.org/hudson/job/Apache%20Wicket%201.5.x/511/])
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Reopened: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg reopened WICKET-3166:
-----------------------------------
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, test-WICKET-3166.patch, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12931934#action_12931934 ]
Hudson commented on WICKET-3166:
--------------------------------
Integrated in Apache Wicket 1.4.x #266 (See [https://hudson.apache.org/hudson/job/Apache%20Wicket%201.4.x/266/])
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Pedro Santos (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pedro Santos updated WICKET-3166:
---------------------------------
Attachment: test-for-unnecessary-calls-WICKET-3166.patch
Sending the test that I would like to see passing.
@Igor I don't think so. On currently impl. the anyEmbeddedMultipart[0] is only assigned to true. If for some reason it needed to respect some very special children rule, we would see some false assignment also.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, test-for-unnecessary-calls-WICKET-3166.patch, test-WICKET-3166.patch, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Martin Makundi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12931876#action_12931876 ]
Martin Makundi commented on WICKET-3166:
----------------------------------------
Hi!
The fix I proposed does fix the probelm at hand but it makes page rendering tremendeously slow due to the recursion:
Component componentParent = component.getParent();
is a recursive call that will be called O(n^n) times in hierarcy.
Something else must be proposed to fix this? The isEnabledInHierarchty uses a caching mechanism. Maybe this will also need to cache the "isparentvisible" at each level thus avoiding O(n^n) recursion.
Any suggestions how to speed it up?
**
Maritin
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12932801#action_12932801 ]
Igor Vaynberg commented on WICKET-3166:
---------------------------------------
i am thinking we should revert this in 1.4.x branch because it may potentially break existing apps.
in 1.5.x we should leave it as it is and add caching like we do for isenabledinhierarchy().
when it comes to visibility and enableness it is better to ask the parent first. the contract is that the child cant be visible/enabled if the parent is not, so it makes sense to ask the parent first in case there is code in child that depends on the parent.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, test-WICKET-3166.patch, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg resolved WICKET-3166.
-----------------------------------
Resolution: Fixed
Fix Version/s: 1.5-M4
1.4.14
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12932845#action_12932845 ]
Igor Vaynberg commented on WICKET-3166:
---------------------------------------
@pedro: post-order is there for a reason. we need to have form-workflow stepped on children invoked before parents so formcomponentpanels work. not sure if we need post order on multipart stuff.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, test-WICKET-3166.patch, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg reassigned WICKET-3166:
-------------------------------------
Assignee: Igor Vaynberg
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Pedro Santos (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12932757#action_12932757 ]
Pedro Santos commented on WICKET-3166:
--------------------------------------
My suggested fix would involve in not visiting form components in post order, which is no a good idea.
Martin, the FormComponent#visitComponentsPostOrderHelper method defines how deep the visitor should go testing: IFormVisitorParticipant#processChildren. How about implementing the IFormVisitorParticipant interface at the mentioned panel1, and return false when it is not visible?
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Martin Makundi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12932831#action_12932831 ]
Martin Makundi commented on WICKET-3166:
----------------------------------------
The original isvisibleinhierarcy was breaking our production application (attached test case).
The slightly improved isvisibleinhierarcy first checks parent visibility, this is ok and works currently ok in our experience (is applied to our production environment since monday; no errors).
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, test-WICKET-3166.patch, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Martin Makundi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Martin Makundi updated WICKET-3166:
-----------------------------------
Attachment: Wicket-Quickstart.zip
junit test case to Reproduce the issue.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Attachments: Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Martin Makundi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12931879#action_12931879 ]
Martin Makundi commented on WICKET-3166:
----------------------------------------
Hah..
actually there is an unnecessary recursion in the proposal. The while-loop is no longer needed if parent is already called.
**
Martin
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Martin Makundi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12931986#action_12931986 ]
Martin Makundi commented on WICKET-3166:
----------------------------------------
1. THIS SOLUTION WORKS
:::
public final boolean isVisibleInHierarchy()
{
Component componentParent = getParent();
return ((componentParent == null) || componentParent.isVisibleInHierarchy()) && determineVisibility();
}
:::
2. CACHING HOWEVER DOES NOT. I tried making and using metaData -key for visibility but it broke rendering in our production app.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Martin Makundi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12932711#action_12932711 ]
Martin Makundi commented on WICKET-3166:
----------------------------------------
I think changing the form visitor is a totally different topic of its own.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Pedro Santos (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12932709#action_12932709 ]
Pedro Santos commented on WICKET-3166:
--------------------------------------
The currently isVisibleInHierarchy implementation has two recursion simple base cases:
- parent == null
- !parent.isVisibleInHierarchy()
Following this logic, the recursion will always stack up calls until test the component graph root node.
Why don't only change the mentioned form visitor to don't go deeper in components that are not visible?
HtmlHeaderContainer already has an similar iterator in the renderHeaderSections method.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Pedro Santos (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pedro Santos updated WICKET-3166:
---------------------------------
Attachment: test-WICKET-3166.patch
Patch restoring the is enabled/visible in hierarchy method;
Changing the Form#isMultiPart to don't traversal deeper in not visible/enable component hierarchy, plus test case exposing an preventing this;
Martin, please take a look at how I used the IFormVisitorParticipant on the test case. See if it can help u.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, test-WICKET-3166.patch, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12931954#action_12931954 ]
Hudson commented on WICKET-3166:
--------------------------------
Integrated in Apache Wicket 1.4.x #267 (See [https://hudson.apache.org/hudson/job/Apache%20Wicket%201.4.x/267/])
meh. this caching will probably slow things down. and it can be difficult to find all the places in code where caching should be invalidated. lets hold off until it is a hotspot.
Issue: WICKET-3166
caching for isVisibleInHieararchy()
Issue: WICKET-3166
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (WICKET-3166) isVisibleInHierarchy() possibly
unnecessarily checks children whose parents are invisible?
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg resolved WICKET-3166.
-----------------------------------
Resolution: Fixed
caching added to 1.5 branch.
> isVisibleInHierarchy() possibly unnecessarily checks children whose parents are invisible?
> ------------------------------------------------------------------------------------------
>
> Key: WICKET-3166
> URL: https://issues.apache.org/jira/browse/WICKET-3166
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 1.4.13
> Reporter: Martin Makundi
> Assignee: Igor Vaynberg
> Fix For: 1.4.14, 1.5-M4
>
> Attachments: diff.txt, test-for-unnecessary-calls-WICKET-3166.patch, test-WICKET-3166.patch, Wicket-Quickstart.zip
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Hi!
> See attached quickstart with junit test reproducing the bug. See also patch proposal.
> I have a page with two panels:
> page.form.add(panel1);
> page.form.add(panel2);
> in some situations panel1 is not visible.
> However, a form submit event will visit all formcomponents of panel1 via:
> at org.apache.wicket.markup.html.form.FormComponent.visitFormComponentsPostOrder(FormComponent.java:400)
> at org.apache.wicket.markup.html.form.Form.visitFormComponentsPostOrder(Form.java:1209)
> at org.apache.wicket.markup.html.form.Form.inputChanged(Form.java:1403)
> at org.apache.wicket.markup.html.form.Form.onFormSubmitted(Form.java:865)
> This results in a crash because panel1 components are not prepared to
> be invoked via isvisible when the panel itself is not visible.
> I wonder if the component.isVisibleInHierarchy could be changed as
> follows, to first check parent visibility:
> public final boolean isVisibleInHierarchy()
> {
> Component component = this;
> while (component != null)
> {
> Component componentParent = component.getParent();
> if (((componentParent == null) ||
> componentParent.isVisibleInHierarchy()) &&
> component.determineVisibility())
> {
> component = componentParent;
> }
> else
> {
> return false;
> }
> }
> return true;
> }
> Similar change could/should maybe be possible also for isEnabledInHierarchy ?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.