[ 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.