Bug 506615. XUL decks should not have wididgets. r=dbaron

layout/generic/nsContainerFrame.cpp

@@ -617,11 +617,10 @@ nsContainerFrame::SyncFrameViewProperties(nsPresContext*  aPresContext,
 
     if (!aStyleContext->GetStyleVisibility()->IsVisible() &&
         !aFrame->SupportsVisibilityHidden()) {
-      // If it's a scrollable frame that can't hide its scrollbars,
-      // hide the view. This means that child elements can't override
-      // their parent's visibility, but it's not practical to leave it
-      // visible in all cases because the scrollbars will be showing
-      // XXXldb Does the view system really enforce this correctly?
+      // If it's a subdocument frame or a plugin, hide the view and
+      // any associated widget.
+      // These are leaf elements so this is OK, no descendant can be
+      // visibility:visible.
       viewIsVisible = PR_FALSE;
     } else if (IsMenuPopup(aFrame)) {
       // if the view is for a popup, don't show the view if the popup is closed

layout/generic/nsFrame.cpp

@@ -6714,15 +6714,6 @@ nsFrame::SetParent(const nsIFrame* aParent)
   else if (wasBoxWrapped && !IsBoxWrapped())
     DeleteProperty(nsGkAtoms::boxMetricsProperty);
 
-  if (aParent && aParent->IsBoxFrame()) {
-    if (aParent->ChildrenMustHaveWidgets()) {
-        nsHTMLContainerFrame::CreateViewForFrame(this, PR_TRUE);
-        nsIView* view = GetView();
-        if (!view->HasWidget())
-          CreateWidgetForView(view);
-    }
-  }
-
   return NS_OK;
 }
 

layout/generic/nsIFrame.h

@@ -106,11 +106,9 @@ struct nsMargin;
 
 typedef class nsIFrame nsIBox;
 
-// IID for the nsIFrame interface
-// 2871104e-2738-4ad7-b86a-ede63c71f1c2
 #define NS_IFRAME_IID \
-  { 0x2871104e, 0x2738, 0x4ad7,                       \
-    { 0xb8, 0x6a, 0xed, 0xe6, 0x3c, 0x71, 0xf1, 0xc2 } }
+  { 0x7e9018b5, 0x5405, 0x4e2b, \
+    { 0x87, 0x67, 0xe2, 0xb4, 0xb1, 0x3e, 0xc1, 0x69 } }
 
 /**
  * Indication of how the frame can be split. This is used when doing runaround
@@ -2290,9 +2288,6 @@ NS_PTR_TO_INT32(frame->GetProperty(nsGkAtoms::embeddingLevel))
   NS_IMETHOD DumpBox(FILE* out)=0;
 #endif
 
-  // Only nsDeckFrame requires that all its children have widgets
-  virtual PRBool ChildrenMustHaveWidgets() const { return PR_FALSE; }
-
   /**
    * @return PR_TRUE if this text frame ends with a newline character.  It
    * should return PR_FALSE if this is not a text frame.

layout/xul/base/src/nsBoxFrame.cpp

@@ -205,18 +205,6 @@ nsBoxFrame::Init(nsIContent*      aContent,
 
   MarkIntrinsicWidthsDirty();
 
-  // see if we need a widget
-  if (aParent && aParent->IsBoxFrame()) {
-    if (aParent->ChildrenMustHaveWidgets()) {
-        rv = nsHTMLContainerFrame::CreateViewForFrame(this, PR_TRUE);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        nsIView* view = GetView();
-        if (!view->HasWidget())
-           view->CreateWidget(kWidgetCID);   
-    }
-  }
-
   CacheAttributes();
 
 #ifdef DEBUG_LAYOUT

layout/xul/base/src/nsDeckFrame.cpp

@@ -56,6 +56,7 @@
 #include "nsBoxLayoutState.h"
 #include "nsStackLayout.h"
 #include "nsDisplayList.h"
+#include "nsHTMLContainerFrame.h"
 
 nsIFrame*
 NS_NewDeckFrame(nsIPresShell* aPresShell, nsStyleContext* aContext)
@@ -107,6 +108,39 @@ nsDeckFrame::Init(nsIContent*     aContent,
   return rv;
 }
 
+static void
+CreateViewsForFrames(const nsFrameList& aFrames)
+{
+  for (nsFrameList::Enumerator f(aFrames); !f.AtEnd(); f.Next()) {
+    nsHTMLContainerFrame::CreateViewForFrame(f.get(), PR_TRUE);
+  }
+}
+
+NS_IMETHODIMP
+nsDeckFrame::SetInitialChildList(nsIAtom*        aListName,
+                                 nsFrameList&    aChildList)
+{
+  CreateViewsForFrames(aChildList);
+  return nsBoxFrame::SetInitialChildList(aListName, aChildList);
+}
+
+NS_IMETHODIMP
+nsDeckFrame::AppendFrames(nsIAtom*        aListName,
+                          nsFrameList&    aFrameList)
+{
+  CreateViewsForFrames(aFrameList);
+  return nsBoxFrame::AppendFrames(aListName, aFrameList);
+}
+
+NS_IMETHODIMP
+nsDeckFrame::InsertFrames(nsIAtom*        aListName,
+                          nsIFrame*       aPrevFrame,
+                          nsFrameList&    aFrameList)
+{
+  CreateViewsForFrames(aFrameList);
+  return nsBoxFrame::InsertFrames(aListName, aPrevFrame, aFrameList);
+}
+
 void
 nsDeckFrame::HideBox(nsPresContext* aPresContext, nsIBox* aBox)
 {

layout/xul/base/src/nsDeckFrame.h

@@ -73,11 +73,17 @@ public:
                   nsIFrame*        aParent,
                   nsIFrame*        aPrevInFlow);
 
-  virtual nsIAtom* GetType() const;
+  // Override SetInitialChildList/AppendFrames/InsertFrames to
+  // create views for our child frames if necessary
+  NS_IMETHOD  SetInitialChildList(nsIAtom*        aListName,
+                                  nsFrameList&    aChildList);
+  NS_IMETHOD AppendFrames(nsIAtom*        aListName,
+                          nsFrameList&    aFrameList);
+  NS_IMETHOD InsertFrames(nsIAtom*        aListName,
+                          nsIFrame*       aPrevFrame,
+                          nsFrameList&    aFrameList);
 
-#ifndef MOZ_GFX_OPTIMIZE_MOBILE
-  virtual PRBool ChildrenMustHaveWidgets() const { return PR_TRUE; }
-#endif
+  virtual nsIAtom* GetType() const;
 
 #ifdef NS_DEBUG
   NS_IMETHOD GetFrameName(nsAString& aResult) const

layout/xul/base/src/nsLeafBoxFrame.cpp

@@ -98,18 +98,6 @@ nsLeafBoxFrame::Init(
   nsresult  rv = nsLeafFrame::Init(aContent, aParent, aPrevInFlow);
   NS_ENSURE_SUCCESS(rv, rv);
 
-   // see if we need a widget
-  if (aParent && aParent->IsBoxFrame()) {
-    if (aParent->ChildrenMustHaveWidgets()) {
-        rv = nsHTMLContainerFrame::CreateViewForFrame(this, PR_TRUE); 
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        nsIView* view = GetView();
-        if (!view->HasWidget())
-           view->CreateWidget(kWidgetCID);   
-    }
-  }
-  
   mMouseThrough = unset;
 
   UpdateMouseThrough();

view/public/nsIViewManager.h

@@ -256,7 +256,12 @@ public:
                          PRBool aRepaintExposedAreaOnly = PR_FALSE) = 0;
 
   /**
-   * Set the visibility of a view.
+   * Set the visibility of a view. Hidden views have the effect of hiding
+   * their descendants as well. This does not affect painting, so layout
+   * is responsible for ensuring that content in hidden views is not
+   * painted nor handling events. It does affect the visibility of widgets;
+   * if a view is hidden, descendant views with widgets have their widgets
+   * hidden.
    * The view manager generates the appropriate dirty regions.
    * @param aView view to change visibility state of
    * @param visible new visibility state

view/src/nsView.cpp

@@ -52,7 +52,6 @@ static nsEventStatus HandleEvent(nsGUIEvent *aEvent);
 
 
 //#define SHOW_VIEW_BORDERS
-//#define HIDE_ALL_WIDGETS
 
 // {34297A07-A8FD-d811-87C6-000244212BCB}
 #define VIEW_WRAPPER_IID \
@@ -338,6 +337,15 @@ void nsView::ResetWidgetBounds(PRBool aRecurse, PRBool aMoveOnly,
   }
 }
 
+PRBool nsView::IsEffectivelyVisible()
+{
+  for (nsView* v = this; v; v = v->mParent) {
+    if (v->GetVisibility() == nsViewVisibility_kHide)
+      return PR_FALSE;
+  }
+  return PR_TRUE;
+}
+
 nsIntRect nsView::CalcWidgetBounds(nsWindowType aType)
 {
   nsCOMPtr<nsIDeviceContext> dx;
@@ -354,7 +362,7 @@ nsIntRect nsView::CalcWidgetBounds(nsWindowType aType)
     viewBounds += offset;
 
     if (parentWidget && aType == eWindowType_popup &&
-        mVis == nsViewVisibility_kShow) {
+        IsEffectivelyVisible()) {
       nsIntPoint screenPoint = parentWidget->WidgetToScreenOffset();
       viewBounds += nsPoint(NSIntPixelsToAppUnits(screenPoint.x, p2a),
                             NSIntPixelsToAppUnits(screenPoint.y, p2a));
@@ -433,29 +441,38 @@ void nsView::SetDimensions(const nsRect& aRect, PRBool aPaint, PRBool aResizeWid
   }
 }
 
-NS_IMETHODIMP nsView::SetVisibility(nsViewVisibility aVisibility)
+void nsView::NotifyEffectiveVisibilityChanged(PRBool aEffectivelyVisible)
 {
-
-  mVis = aVisibility;
-
-  if (aVisibility == nsViewVisibility_kHide)
+  if (!aEffectivelyVisible)
   {
     DropMouseGrabbing();
   }
 
   if (nsnull != mWindow)
   {
-#ifndef HIDE_ALL_WIDGETS
-    if (mVis == nsViewVisibility_kShow)
+    if (aEffectivelyVisible)
     {
       DoResetWidgetBounds(PR_FALSE, PR_TRUE);
       mWindow->Show(PR_TRUE);
     }
     else
-#endif
       mWindow->Show(PR_FALSE);
   }
 
+  for (nsView* child = mFirstChild; child; child = child->mNextSibling) {
+    if (child->mVis == nsViewVisibility_kHide) {
+      // It was effectively hidden and still is
+      continue;
+    }
+    // Our child is visible if we are
+    child->NotifyEffectiveVisibilityChanged(aEffectivelyVisible);
+  }
+}
+
+NS_IMETHODIMP nsView::SetVisibility(nsViewVisibility aVisibility)
+{
+  mVis = aVisibility;
+  NotifyEffectiveVisibilityChanged(IsEffectivelyVisible());
   return NS_OK;
 }
 

view/src/nsView.h

@@ -166,6 +166,8 @@ public:
   void SetPositionIgnoringChildWidgets(nscoord aX, nscoord aY);
   nsresult LoadWidget(const nsCID &aClassIID);
 
+  void NotifyEffectiveVisibilityChanged(PRBool aEffectivelyVisible);
+
   // Update the cached RootViewManager for all view manager descendents,
   // If the hierarchy is being removed, aViewManagerParent points to the view
   // manager for the hierarchy's old parent, and will have its mouse grab
@@ -193,6 +195,8 @@ public:
 
   nsIntRect CalcWidgetBounds(nsWindowType aType);
 
+  PRBool IsEffectivelyVisible();
+
 protected:
   // Do the actual work of ResetWidgetBounds, unconditionally.  Don't
   // call this method if we have no widget.

view/src/nsViewManager.cpp

@@ -110,14 +110,8 @@ public:
 
 static PRBool IsViewVisible(nsView *aView)
 {
-  for (nsIView *view = aView; view; view = view->GetParent()) {
-    // We don't check widget visibility here because in the future (with
-    // the better approach to this that's in attachment 160801 on bug
-    // 227361), callers of the equivalent to this function should be able
-    // to rely on being notified when the result of this function changes.
-    if (view->GetVisibility() == nsViewVisibility_kHide)
+  if (!aView->IsEffectivelyVisible())
     return PR_FALSE;
-  }
 
   // Find out if the root view is visible by asking the view observer
   // (this won't be needed anymore if we link view trees across chrome /
@@ -1231,8 +1225,7 @@ NS_IMETHODIMP nsViewManager::GrabMouseEvents(nsIView *aView, PRBool &aResult)
 
   // Along with nsView::SetVisibility, we enforce that the mouse grabber
   // can never be a hidden view.
-  if (aView && static_cast<nsView*>(aView)->GetVisibility()
-               == nsViewVisibility_kHide) {
+  if (aView && !static_cast<nsView*>(aView)->IsEffectivelyVisible()) {
     aView = nsnull;
   }
 
@@ -1587,16 +1580,6 @@ NS_IMETHODIMP nsViewManager::SetViewVisibility(nsIView *aView, nsViewVisibility
         }
       }
     }
-
-    // Any child views not associated with frames might not get their visibility
-    // updated, so propagate our visibility to them. This is important because
-    // hidden views should have all hidden children.
-    for (nsView* childView = view->GetFirstChild(); childView;
-         childView = childView->GetNextSibling()) {
-      if (!childView->GetClientData()) {
-        childView->SetVisibility(aVisible);
-      }
-    }
   }
   return NS_OK;
 }
@@ -1967,7 +1950,7 @@ NS_IMETHODIMP nsViewManager::GetRectVisibility(nsIView *aView,
   }
 
   // is this view even visible?
-  if (view->GetVisibility() == nsViewVisibility_kHide) {
+  if (!view->IsEffectivelyVisible()) {
     return NS_OK; 
   }