modal works like a charm now

src/client.cc

@@ -44,7 +44,6 @@ Client::Client(int screen, Window window)
   _urgent = false;
   _positioned = false;
   _disabled_decorations = 0;
-  _modal_child = 0;
   _group = None;
   _desktop = 0;
   
@@ -670,18 +669,11 @@ void Client::updateTransientFor()
 
   // if anything has changed...
   if (c != _transient_for) {
-    bool m = _modal;
-    if (_modal)
-      setModal(false);
-    
     if (_transient_for)
       _transient_for->_transients.remove(this); // remove from old parent
     _transient_for = c;
     if (_transient_for)
       _transient_for->_transients.push_back(this); // add to new parent
-
-    if (m)
-      setModal(true);
   }
 }
 
@@ -794,65 +786,12 @@ void Client::setDesktop(long target)
 }
 
 
-Client *Client::findModalChild(Client *skip) const
-{
-  Client *ret = 0;
-  
-  // find a modal child recursively and try focus it
-  List::const_iterator it, end = _transients.end();
-  for (it = _transients.begin(); it != end; ++it)
-    if ((*it)->_modal && *it != skip)
-      return *it; // got one
-  // none of our direct children are modal, let them try check
-  for (it = _transients.begin(); it != end; ++it)
-    if ((ret = (*it)->findModalChild()))
-      return ret; // got one
-  return ret;
-}
-
-
-void Client::setModal(bool modal)
-{
-  if (modal == _modal) return;
-  
-  if (modal) {
-    Client *c = this;
-    while (c->_transient_for) {
-      c = c->_transient_for;
-      if (c == this) break; // circular?
-      if (c->_modal_child) break; // already has a modal child
-      c->_modal_child = this;
-    }
-  } else {
-    // try find a replacement modal dialog
-    Client *replacement = 0;
-    
-    Client *c = this;
-    while (c->_transient_for) // go up the tree
-      c = c->_transient_for;
-    replacement = c->findModalChild(this); // find a modal child, skipping this
-    assert(replacement != this);
-
-    c = this;
-    while (c->_transient_for) {
-      c = c->_transient_for;
-      if (c == this) break; // circular?
-      if (c->_modal_child != this) break; // has a different modal child
-      if (c == replacement) break; // found the replacement itself
-      c->_modal_child = replacement;
-    }
-  }
-  _modal = modal;
-}
-
-
 void Client::setState(StateAction action, long data1, long data2)
 {
   bool shadestate = _shaded;
   bool fsstate = _fullscreen;
   bool maxh = _max_horz;
   bool maxv = _max_vert;
-  bool modal = _modal;
 
   if (!(action == State_Add || action == State_Remove ||
         action == State_Toggle))
@@ -888,7 +827,7 @@ void Client::setState(StateAction action, long data1, long data2)
     if (action == State_Add) {
       if (state == otk::Property::atoms.net_wm_state_modal) {
         if (_modal) continue;
-        modal = true;
+        _modal = true;
       } else if (state == otk::Property::atoms.net_wm_state_maximized_vert) {
         maxv = true;
       } else if (state == otk::Property::atoms.net_wm_state_maximized_horz) {
@@ -913,7 +852,7 @@ void Client::setState(StateAction action, long data1, long data2)
     } else { // action == State_Remove
       if (state == otk::Property::atoms.net_wm_state_modal) {
         if (!_modal) continue;
-        modal = false;
+        _modal = false;
       } else if (state == otk::Property::atoms.net_wm_state_maximized_vert) {
         maxv = false;
       } else if (state == otk::Property::atoms.net_wm_state_maximized_horz) {
@@ -950,8 +889,6 @@ void Client::setState(StateAction action, long data1, long data2)
         maximize(maxv, 2, true);
     }
   }
-  if (modal != _modal)
-    setModal(modal);
   // change fullscreen state before shading, as it will affect if the window
   // can shade or not
   if (fsstate != _fullscreen)
@@ -1382,11 +1319,6 @@ void Client::applyStartupState()
 {
   // these are in a carefully crafted order..
 
-  if (_modal) {
-    _modal = false;
-    setModal(true);
-  }
-  
   if (_iconic) {
     _iconic = false;
     setDesktop(ICONIC_DESKTOP);
@@ -1644,11 +1576,38 @@ void Client::installColormap(bool install) const
 }
 
 
+// recursively searches the client 'tree' for a modal client, always skips the
+// topmost node (the window you're starting with)
+Client *Client::searchModalTree(Client *node, Client *skip)
+{
+  List::const_iterator it, end = node->_transients.end();
+  Client *ret;
+  
+  for (it = node->_transients.begin(); it != end; ++it) {
+    if (*it == skip) continue; // circular?
+    printf("recursing\n");
+    if ((ret = searchModalTree(*it, skip))) return ret; // got one
+    printf("trying this window\n");
+    if ((*it)->_modal) {
+          printf("found it\n");
+          return *it; // got one
+    }
+  }
+  printf("found none\n");
+  return 0;
+}
+
+Client *Client::findModalChild()
+{
+  return searchModalTree(this, this);
+}
+
+
 bool Client::focus()
 {
   // if we have a modal child, then focus it, not us
-  if (_modal_child)
+  Client *c = findModalChild();
-    return _modal_child->focus();
+  if (c) return c->focus();
 
   // won't try focus if the client doesn't want it, or if the window isn't
   // visible on the screen

src/client.hh

@@ -295,9 +295,6 @@ private:
   //! The window uses shape extension to be non-rectangular?
   bool _shaped;
 
-  //! If the window has a modal child window, then this will point to it
-  Client *_modal_child;
-
   //! The window is modal, so it must be processed before any windows it is
   //! related to can be focused
   bool _modal;
@@ -384,11 +381,6 @@ private:
     by sending it to any other valid desktop.
   */
   void setDesktop(long desktop);
-  //! Set whether the window is modal or not
-  /*!
-    This adjusts references in parents etc to match.
-  */
-  void setModal(bool modal);
   
   //! Calculates the stacking layer for the client window
   void calcLayer();
@@ -431,6 +423,10 @@ private:
   */
   void shade(bool shade);
 
+  //! Recursively searches the client 'tree' for a modal client, always skips
+  //! the topmost node (the window you're starting with).
+  Client *Client::searchModalTree(Client *node, Client *skip);
+
   //! Fires the urgent callbacks which lets the user do what they want with
   //! urgent windows
   void fireUrgent();
@@ -482,9 +478,6 @@ private:
   void internal_resize(Corner anchor, int w, int h,
                        bool user = true, int x = INT_MIN, int y = INT_MIN);
 
-  //! Attempts to find and return a modal child of this window, recursively.
-  Client *findModalChild(Client *skip = 0) const;
-
   //! Removes or reapplies the client's border to its window
   /*!
     Used when managing and unmanaging a window.
@@ -587,9 +580,6 @@ BB    @param window The window id that the Client class should handle
   //! Return the client this window is transient for
   inline Client *transientFor() const { return _transient_for; }
 
-  //! Returns the window which is a modal child of this window
-  inline Client *modalChild() const { return _modal_child; }
-  
   //! Returns if the window is modal
   /*!
     If the window is modal, then no other windows that it is related to can get
@@ -670,6 +660,12 @@ BB    @param window The window id that the Client class should handle
   */
   void disableDecorations(DecorationFlags flags);
   
+  //! Return a modal child of the client window
+  /*!
+    @return A modal child of the client window, or 0 if none was found.
+  */
+  Client *findModalChild();
+
   //! Attempt to focus the client window
   bool focus();
 

src/screen.cc

@@ -595,7 +595,7 @@ void Screen::unmanageWindow(Client *client)
   updateStrut();
 
   // unset modal before dropping our focus
-  client->setModal(false);
+  client->_modal = false;
   
   // unfocus the client (calls the focus callbacks)
   client->unfocus();
@@ -652,6 +652,10 @@ void Screen::raiseWindow(Client *client)
 
   assert(!_stacking.empty()); // this would be bad
 
+  Client *m = client->findModalChild();
+  // if we have a modal child, raise it instead, we'll go along tho later
+  if (m) raiseWindow(m);
+  
   // remove the client before looking so we can't run into ourselves
   _stacking.remove(client);
   
@@ -659,7 +663,10 @@ void Screen::raiseWindow(Client *client)
   const ClientList::iterator end = _stacking.end();
 
   // the stacking list is from highest to lowest
-  for (; it != end && (*it)->layer() > client->layer(); ++it);
+//  for (;it != end, ++it) {
+//    if ((*it)->layer() <= client->layer() && m != *it) break;
+//  }
+  for (; it != end && ((*it)->layer() > client->layer() || m == *it); ++it);
 
   /*
     if our new position is the top, we want to stack under the _focuswindow
@@ -673,11 +680,7 @@ void Screen::raiseWindow(Client *client)
 
   XRestackWindows(**otk::display, wins, 2);
 
-  // if the window has a modal child, then raise it after us to put it on top
+  changeStackingList(); 
-  if (client->modalChild())
-    raiseWindow(client->modalChild());
-  else
-    changeStackingList(); // no need to do this twice!
 }
 
 void Screen::changeDesktop(long desktop)