Bug 114342 - ModalPanel needs a code review
Summary: ModalPanel needs a code review
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Web Application Framework
Classification: Retired
Component: other
Version: nightly
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: ccm-bugs-list
QA Contact: Jon Orris
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2004-01-26 22:14 UTC by Vadim Nasardinov
Modified: 2007-04-18 17:02 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-08-03 18:43:26 UTC
Embargoed:


Attachments (Terms of Use)

Description Vadim Nasardinov 2004-01-26 22:14:50 UTC
ModalPanel manipulates visibility of its child components in a
non-intuitive way.  Let me describe the problem in terms of bug
114313.

The offending piece of code was in LifecycleItemPane and looked like
so:

$ p4 print \
  //cms/dev/src/com/arsdigita/cms/ui/lifecycle/LifecycleItemPane.java#9 |\
  grep -v //cms | nl -ba | head -n 189 | tail -n 10

   180	                    public final void actionPerformed(final ActionEvent e) {
   181	                        final PageState state = e.getPageState();
   182	
   183	                        if (state.isVisibleOnPage(LifecycleItemPane.this)
   184	                                && !hasAdmin(state)) {
   185	                            getColumn(4).setVisible(state, false);
   186	                            getColumn(5).setVisible(state, false);
   187	                        }
   188	                    }
   189	                });


where column #4 is the "Edit phase" link and column #5 the "Delete
phase" link.  So, on the face of it, we *were* trying to hide the
links, if the user did not have the appropriate privilege.  As an
optimization to avoid the permission check, we first checked whether
the LifecycleItemPane was visible on the page.  If not, there would be
no point in making the links invisible.

The problem was that state.isVisibleOnPage(LifecycleItemPane.this) was
returning false, even though the pane *was* visible on the page.
Instrumenting the VisibilityTraversal class in PageState with log4j
statements showed that the parent container of LifecycleItemPane was
ModalPanel.  (Hierarchy-wise, LifecycleItemPane is a child of
LifecycleAdminPane, which is a subclass of SelectionPanel, which uses
ModalPanel as its underlying container.)

If you look at VisibilityListener in ModalPanel, you'll notice that
ModalPanel makes all of its children invisible in the "ActionListener"
phase of the bebop request processing pipeline.  In the "generateXML"
phase of the bebop pipeline, ModalPanel makes its "top" child visible.

So, the problem is that ModalPanel misreports the visibility of some
of its children until bebop reaches the "generateXML" phase.
LifecycleItemPane makes visibility checks during the "ActionListener"
phase.  Prior to change 39772, LifecycleItemPane failed to perform
certain required actions, due to incorrectly reported visibility
settings.

I don't know if there are any other bugs caused by ModalPanel's
current behavior, but I wouldn't be surprised if there are.  We can't
document this issue away, because you generally don't know whether a
particular class uses ModalPanel in its implementation or not.

Comment 1 Vadim Nasardinov 2005-08-03 18:43:26 UTC
slate


Note You need to log in before you can comment on or make changes to this bug.