Bug 114342 - ModalPanel needs a code review
ModalPanel needs a code review
Status: CLOSED WONTFIX
Product: Red Hat Web Application Framework
Classification: Retired
Component: other (Show other bugs)
nightly
All Linux
medium Severity medium
: ---
: ---
Assigned To: ccm-bugs-list
Jon Orris
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2004-01-26 17:14 EST by Vadim Nasardinov
Modified: 2007-04-18 13:02 EDT (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-08-03 14:43:26 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Vadim Nasardinov 2004-01-26 17:14:50 EST
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 14:43:26 EDT
slate

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