loadItem Bug: wrong m_pDocContainer is set

[Daniel Teske]

Hi


While testing my fix for bug 96539, i've come across a more serious
problem:

if m_pDocContainer() is 0L and loadViewProfile() is called, 
the following code is used inside loadItem() selects m_pDocContainer:
(reformated to fit):

if( type == "View" 
   && !m_pDocContainer 
   && parent->frameType() == "Container") 
{  
   KonqFrameContainer* parentContainer =
      static_cast<KonqFrameContainer*>(parent);
   KonqFrameBase* otherFrame =
      parentContainer->otherChild( childView->frame() );
   if (otherFrame)
   {
     if (childView->isPassiveMode())
     {
       if (otherFrame->frameType() == "View")
       {
         KonqFrame* viewFrame = static_cast<KonqFrame*>(otherFrame);
         if (viewFrame->childView()->isPassiveMode())
           m_pDocContainer = parentContainer; // Both views are passive,
                                              // shouldn't happen
         else
           m_pDocContainer = viewFrame; // This one is passive, 
                                        //  the other is active
       }
     }
     else
     {
       if (otherFrame->frameType() == "View")
       {
         KonqFrame* viewFrame = static_cast<KonqFrame*>(otherFrame);
         if (viewFrame->childView()->isPassiveMode())
           m_pDocContainer = childView->frame(); // This one is active, 
                                                 //  the other is
passive
         else
           m_pDocContainer = parentContainer; // Both views are active
         }
         else
           m_pDocContainer = parentContainer; // This one is active, 
                                              // the other is a
Container
         }
       }
     }
   }
}

Basically:
If we have loaded the other child: 
if one is passive, set the m_pDocContainer = other 
else if both are active or one is active and the other is a container
m_pDocContainer = parent of both

This code fails if there are two layers of KonqFrameContainers, e.g.
something like this:
(All views active.)

MainWindow
|>Container0
  |>Container1
    |>View3
    |>View4
  |>Container2
    |>View5
    |>View6

Because loadItem(View4) sets m_pDocContainer to Container 1.

One way to demonstrate the bug is by opening a new Konqueror, splitting
the view in a upper and lower half. Splitting both the upper and lower
half, and then detaching the tab.
It is a little bit harder but, possible to create profile, which has
such a structure.

This patch fixes that, but I'm not that it doesn't break anything else
and that I thought about all possible profiles.

The basic idea is:
if m_pDocContainer is 0L, every container asks its children if they have
only active children, this ensures that in the example above
m_pDocContainer is set to Container0.
The way a children c communicates that it has only active children is by
setting m_pDocContainer = c;

If the parent sees that the first child has set m_pDocContainer to
itsself, m_pDocContainer must have been 0L before[1], so we ask the
second children if they have only active children by setting
m_pDocContainer to 0L.
if both want, then the parent sets m_pDocContainer to itself, if only
one children wants it, it takes it.

So in the case above:
|>Container0
  |>Container1
    |>View3
     m_pDocContainer is 0L, so view 3 sets m_pDocContainer = view3
   container 1 sees view3 has set m_pDocContainer, so resets to 0L
    |>View4
     m_pDocContainer is 0L, so view 4 sets m_pDocContainer = view4
   Both children of Container1 want to be m_pDocContainer 
   so Container 1 sets m_pDocContainer = Container 1
 container 0 sees that container 1 has set m_pDocContainer so
 it sets m_pDocContainer = 0L and asks container 2.
  |>Container2
    |>View5
     m_pDocContainer is 0L, so view 5 sets m_pDocContainer = view5
   container 2 sees view5 has set m_pDocContainer, so resets to 0L
    |>View6
     m_pDocContainer is 0L, so view 6 sets m_pDocContainer = view6
   Both children of Container2 want to be m_pDocContainer 
   so Container 2 sets m_pDocContainer = Container 2
 Both children of Container0 want to be m_pDocContainer 
 so Container 0 sets m_pDocContainer = Container 0

I thought about a lot of other trees. (Obviously some with a tabbar and
some with passive views.) I think that this code is correct, but I don't
know if I thought of everything.

I would be grateful, if someone with more knowledge of that area would
take a look at this patch/problem.

daniel
[1] Not strictly true, there are a few other cases where a child sets
m_pDocContainer, I think that they can be handled the same way.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20050122/918f08cd/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixloadItem_DocContainer.patch
Type: text/x-patch
Size: 3065 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20050122/918f08cd/attachment.bin>