Bug 575294. part=2/4 r=smaug,roc

layout/forms/nsComboboxControlFrame.cpp

@@ -336,7 +336,6 @@ nsComboboxControlFrame::SetFocus(bool aOn, bool aRepaint)
       if (!weakFrame.IsAlive()) {
         return;
       }
-      MOZ_ASSERT(!mDelayedShowDropDown);
     }
   } else {
     mFocused = nsnull;
@@ -450,6 +449,29 @@ nsComboboxControlFrame::ShowList(bool aShowList)
   return weakFrame.IsAlive();
 }
 
+class nsResizeDropdownAtFinalPosition : public nsIReflowCallback
+{
+public:
+  nsResizeDropdownAtFinalPosition(nsComboboxControlFrame* aFrame)
+    : mFrame(aFrame) {}
+
+  virtual bool ReflowFinished()
+  {
+    if (mFrame.IsAlive()) {
+      static_cast<nsComboboxControlFrame*>(mFrame.GetFrame())->
+        AbsolutelyPositionDropDown();
+    }
+    return false;
+  }
+
+  virtual void ReflowCallbackCanceled()
+  {
+    delete this;
+  }
+
+  nsWeakFrame mFrame;
+};
+
 nsresult
 nsComboboxControlFrame::ReflowDropdown(nsPresContext*  aPresContext, 
                                        const nsHTMLReflowState& aReflowState)
@@ -543,49 +565,149 @@ nsComboboxControlFrame::GetCSSTransformTranslation()
   return translation;
 }
 
-void
-nsComboboxControlFrame::AbsolutelyPositionDropDown()
+class nsAsyncRollup : public nsRunnable
 {
-   // Position the dropdown list. It is positioned below the display frame if there is enough
-   // room on the screen to display the entire list. Otherwise it is placed above the display
-   // frame.
+public:
+  nsAsyncRollup(nsComboboxControlFrame* aFrame) : mFrame(aFrame) {}
+  NS_IMETHODIMP Run()
+  {
+    if (mFrame.IsAlive()) {
+      static_cast<nsComboboxControlFrame*>(mFrame.GetFrame())
+        ->RollupFromList();
+    }
+    return NS_OK;
+  }
+  nsWeakFrame mFrame;
+};
 
-   // Note: As first glance, it appears that you could simply get the absolute bounding box for the
-   // dropdown list by first getting its view, then getting the view's nsIWidget, then asking the nsIWidget
-   // for it's AbsoluteBounds. The problem with this approach, is that the dropdown lists y location can
-   // change based on whether the dropdown is placed below or above the display frame.
-   // The approach, taken here is to get use the absolute position of the display frame and use it's location
-   // to determine if the dropdown will go offscreen.
+class nsAsyncResize : public nsRunnable
+{
+public:
+  nsAsyncResize(nsComboboxControlFrame* aFrame) : mFrame(aFrame) {}
+  NS_IMETHODIMP Run()
+  {
+    if (mFrame.IsAlive()) {
+      nsComboboxControlFrame* combo =
+        static_cast<nsComboboxControlFrame*>(mFrame.GetFrame());
+      static_cast<nsListControlFrame*>(combo->mDropdownFrame)->
+        SetSuppressScrollbarUpdate(true);
+      nsCOMPtr<nsIPresShell> shell = mFrame->PresContext()->PresShell();
+      shell->FrameNeedsReflow(combo->mDropdownFrame, nsIPresShell::eResize,
+                              NS_FRAME_IS_DIRTY);
+      shell->FlushPendingNotifications(Flush_Layout);
+      if (mFrame.IsAlive()) {
+        combo = static_cast<nsComboboxControlFrame*>(mFrame.GetFrame());
+        static_cast<nsListControlFrame*>(combo->mDropdownFrame)->
+          SetSuppressScrollbarUpdate(false);
+        if (combo->mDelayedShowDropDown) {
+          combo->ShowDropDown(true);
+        }
+      }
+    }
+    return NS_OK;
+  }
+  nsWeakFrame mFrame;
+};
+
+void
+nsComboboxControlFrame::GetAvailableDropdownSpace(nscoord* aAbove,
+                                                  nscoord* aBelow,
+                                                  nsPoint* aTranslation)
+{
+  // Note: As first glance, it appears that you could simply get the absolute
+  // bounding box for the dropdown list by first getting its view, then getting
+  // the view's nsIWidget, then asking the nsIWidget for its AbsoluteBounds.
+  // The problem with this approach, is that the dropdown lists y location can
+  // change based on whether the dropdown is placed below or above the display
+  // frame.  The approach, taken here is to get the absolute position of the
+  // display frame and use its location to determine if the dropdown will go
+  // offscreen.
 
   // Normal frame geometry (eg GetOffsetTo, mRect) doesn't include transforms.
   // In the special case that our transform is only a 2D translation we
   // introduce this hack so that the dropdown will show up in the right place.
-  nsPoint translation = GetCSSTransformTranslation();
-
-   // Use the height calculated for the area frame so it includes both
-   // the display and button heights.
-  nscoord dropdownYOffset = GetRect().height;
-  nsSize dropdownSize = mDropdownFrame->GetSize();
-
+  *aTranslation = GetCSSTransformTranslation();
+  *aAbove = 0;
+  *aBelow = 0;
+  
+  nsRect thisScreenRect = GetScreenRectInAppUnits();
   nsRect screen = nsFormControlFrame::GetUsableScreenRect(PresContext());
+  nscoord dropdownY = thisScreenRect.YMost() + aTranslation->y;
 
-  // Check to see if the drop-down list will go offscreen
-  if ((GetScreenRectInAppUnits() + translation).YMost() + dropdownSize.height > screen.YMost()) {
-    // move the dropdown list up
-    dropdownYOffset = - (dropdownSize.height);
+  nscoord minY;
+  if (!PresContext()->IsChrome()) {
+    nsIFrame* root = PresContext()->PresShell()->GetRootFrame();
+    minY = root->GetScreenRectInAppUnits().y;
+    if (dropdownY < root->GetScreenRectInAppUnits().y) {
+      // Don't allow the drop-down to be placed above the top of the root frame.
+      return;
+    }
+  } else {
+    minY = screen.y;
+  }
+  
+  nscoord below = screen.YMost() - dropdownY;
+  nscoord above = thisScreenRect.y + aTranslation->y - minY;
+
+  // If the difference between the space above and below is less
+  // than a row-height, then we favor the space below.
+  if (above >= below) {
+    nsListControlFrame* lcf = static_cast<nsListControlFrame*>(mDropdownFrame);
+    nscoord rowHeight = lcf->GetHeightOfARow();
+    if (above < below + rowHeight) {
+      above -= rowHeight;
+    }
   }
 
-  nsPoint dropdownPosition;
-  const nsStyleVisibility* vis = GetStyleVisibility();
-  if (vis->mDirection == NS_STYLE_DIRECTION_RTL) {
+  *aBelow = below;
+  *aAbove = above;
+}
+
+nsComboboxControlFrame::DropDownPositionState
+nsComboboxControlFrame::AbsolutelyPositionDropDown()
+{
+  nsPoint translation;
+  nscoord above, below;
+  GetAvailableDropdownSpace(&above, &below, &translation);
+  if (above <= 0 && below <= 0) {
+    // Hide the view immediately to minimize flicker.
+    nsIView* view = mDropdownFrame->GetView();
+    view->GetViewManager()->SetViewVisibility(view, nsViewVisibility_kHide);
+    NS_DispatchToCurrentThread(new nsAsyncRollup(this));
+    return eDropDownPositionSuppressed;
+  }
+
+  nsSize dropdownSize = mDropdownFrame->GetSize();
+  nscoord height = NS_MAX(above, below);
+  nsListControlFrame* lcf = static_cast<nsListControlFrame*>(mDropdownFrame);
+  if (height < dropdownSize.height) {
+    if (lcf->GetNumDisplayRows() > 1) {
+      // The drop-down doesn't fit and currently shows more than 1 row -
+      // schedule a resize to show fewer rows.
+      NS_DispatchToCurrentThread(new nsAsyncResize(this));
+      return eDropDownPositionPendingResize;
+    }
+  } else if (height > (dropdownSize.height + lcf->GetHeightOfARow() * 1.5) &&
+             lcf->GetDropdownCanGrow()) {
+    // The drop-down fits but there is room for at least 1.5 more rows -
+    // schedule a resize to show more rows if it has more rows to show.
+    // (1.5 rows for good measure to avoid any rounding issues that would
+    // lead to a loop of reflow requests)
+    NS_DispatchToCurrentThread(new nsAsyncResize(this));
+    return eDropDownPositionPendingResize;
+  }
+
+  // Position the drop-down below if there is room, otherwise place it
+  // on the side that has more room.
+  bool b = dropdownSize.height <= below || below >= above;
+  nsPoint dropdownPosition(0, b ? GetRect().height : -dropdownSize.height);
+  if (GetStyleVisibility()->mDirection == NS_STYLE_DIRECTION_RTL) {
     // Align the right edge of the drop-down with the right edge of the control.
     dropdownPosition.x = GetRect().width - dropdownSize.width;
-  } else {
-    dropdownPosition.x = 0;
   }
-  dropdownPosition.y = dropdownYOffset; 
-
   mDropdownFrame->SetPosition(dropdownPosition + translation);
+  nsContainerFrame::PositionFrameView(mDropdownFrame);
+  return eDropDownPositionFinal;
 }
 
 //----------------------------------------------------------
@@ -713,6 +835,8 @@ nsComboboxControlFrame::Reflow(nsPresContext*          aPresContext,
 
   // First reflow our dropdown so that we know how tall we should be.
   ReflowDropdown(aPresContext, aReflowState);
+  nsIReflowCallback* cb = new nsResizeDropdownAtFinalPosition(this);
+  aPresContext->PresShell()->PostReflowCallback(cb);
 
   // Get the width of the vertical scrollbar.  That will be the width of the
   // dropdown button.
@@ -803,16 +927,19 @@ nsComboboxControlFrame::ShowDropDown(bool aDoDropDown)
 {
   mDelayedShowDropDown = false;
   nsEventStates eventStates = mContent->AsElement()->State();
-  if (eventStates.HasState(NS_EVENT_STATE_DISABLED)) {
+  if (aDoDropDown && eventStates.HasState(NS_EVENT_STATE_DISABLED)) {
     return;
   }
 
   if (!mDroppedDown && aDoDropDown) {
     if (mFocused == this) {
-      if (mListControlFrame) {
-        mListControlFrame->SyncViewWithFrame();
+      DropDownPositionState state = AbsolutelyPositionDropDown();
+      if (state == eDropDownPositionFinal) {
+        ShowList(aDoDropDown); // might destroy us
+      } else if (state == eDropDownPositionPendingResize) {
+        // Delay until after the resize reflow, see nsAsyncResize.
+        mDelayedShowDropDown = true;
       }
-      ShowList(aDoDropDown); // might destroy us
     } else {
       // Delay until we get focus, see SetFocus().
       mDelayedShowDropDown = true;

layout/forms/nsComboboxControlFrame.h

@@ -137,7 +137,16 @@ public:
    * @note This method might destroy |this|.
    */
   virtual void RollupFromList();
-  virtual void AbsolutelyPositionDropDown();
+
+  /**
+   * Return the available space above and below this frame for
+   * placing the drop-down list, and the current 2D translation.
+   * Note that either or both can be less than or equal to zero,
+   * if both are then the drop-down should be closed.
+   */
+  void GetAvailableDropdownSpace(nscoord* aAbove,
+                                 nscoord* aBelow,
+                                 nsPoint* aTranslation);
   virtual PRInt32 GetIndexOfDisplayArea();
   /**
    * @note This method might destroy |this|.
@@ -184,17 +193,27 @@ public:
   static bool ToolkitHasNativePopup();
 
 protected:
+  friend class RedisplayTextEvent;
+  friend class nsAsyncResize;
+  friend class nsResizeDropdownAtFinalPosition;
 
   // Utilities
   nsresult ReflowDropdown(nsPresContext*          aPresContext, 
                           const nsHTMLReflowState& aReflowState);
 
+  enum DropDownPositionState {
+    // can't show the dropdown at its current position
+    eDropDownPositionSuppressed,
+    // a resize reflow is pending, don't show it yet
+    eDropDownPositionPendingResize,
+    // the dropdown has its final size and position and can be displayed here
+    eDropDownPositionFinal
+  };
+  DropDownPositionState AbsolutelyPositionDropDown();
+
   // Helper for GetMinWidth/GetPrefWidth
   nscoord GetIntrinsicWidth(nsRenderingContext* aRenderingContext,
                             nsLayoutUtils::IntrinsicWidthType aType);
-protected:
-  class RedisplayTextEvent;
-  friend class RedisplayTextEvent;
 
   class RedisplayTextEvent : public nsRunnable {
   public:

layout/forms/nsIComboboxControlFrame.h

@@ -60,11 +60,6 @@ public:
    */
   virtual PRInt32 UpdateRecentIndex(PRInt32 aIndex) = 0;
 
-  /**
-   *
-   */
-  virtual void AbsolutelyPositionDropDown() = 0;
-
   /**
    * Notification that the content has been reset
    */

layout/forms/nsIListControlFrame.h

@@ -61,11 +61,6 @@ public:
 
   virtual PRInt32 GetNumberOfOptions() = 0; 
 
-  /**
-   * 
-   */
-  virtual void SyncViewWithFrame() = 0;
-
   /**
    * Called by combobox when it's about to drop down
    */

layout/forms/nsListControlFrame.cpp

@@ -53,7 +53,7 @@
 using namespace mozilla;
 
 // Constants
-const nscoord kMaxDropDownRows          = 20; // This matches the setting for 4.x browsers
+const PRInt32 kMaxDropDownRows          = 20; // This matches the setting for 4.x browsers
 const PRInt32 kNothingSelected          = -1;
 
 // Static members
@@ -112,6 +112,7 @@ nsListControlFrame::nsListControlFrame(
   : nsHTMLScrollFrame(aShell, aContext, false),
     mMightNeedSecondPass(false),
     mHasPendingInterruptAtStartOfReflow(false),
+    mDropdownCanGrow(false),
     mLastDropdownComputedHeight(NS_UNCONSTRAINEDSIZE)
 {
   mComboboxFrame      = nsnull;
@@ -509,11 +510,12 @@ nsListControlFrame::ReflowAsDropdown(nsPresContext*           aPresContext,
 
 #ifdef DEBUG
   nscoord oldHeightOfARow = HeightOfARow();
+  nscoord oldVisibleHeight = (GetStateBits() & NS_FRAME_FIRST_REFLOW) ?
+    NS_UNCONSTRAINEDSIZE : GetScrolledFrame()->GetSize().height;
 #endif
 
   nsHTMLReflowState state(aReflowState);
 
-  nscoord oldVisibleHeight;
   if (!(GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
     // When not doing an initial reflow, and when the height is auto, start off
     // with our computed height set to what we'd expect our height to be.
@@ -521,11 +523,6 @@ nsListControlFrame::ReflowAsDropdown(nsPresContext*           aPresContext,
     // NS_UNCONSTRAINEDSIZE in cases when last time we didn't have to constrain
     // the height.  That's fine; just do the same thing as last time.
     state.SetComputedHeight(mLastDropdownComputedHeight);
-    oldVisibleHeight = GetScrolledFrame()->GetSize().height;
-  } else {
-    // Set oldVisibleHeight to something that will never test true against a
-    // real height.
-    oldVisibleHeight = NS_UNCONSTRAINEDSIZE;
   }
 
   nsresult rv = nsHTMLScrollFrame::Reflow(aPresContext, aDesiredSize,
@@ -567,48 +564,34 @@ nsListControlFrame::ReflowAsDropdown(nsPresContext*           aPresContext,
   // implementation detail of nsHTMLScrollFrame that we're depending on?
   nsHTMLScrollFrame::DidReflow(aPresContext, &state, aStatus);
 
-  // Now compute the height we want to have
-  mNumDisplayRows = kMaxDropDownRows;
-  if (visibleHeight > mNumDisplayRows * heightOfARow) {
-    visibleHeight = mNumDisplayRows * heightOfARow;
-    // This is an adaptive algorithm for figuring out how many rows 
-    // should be displayed in the drop down. The standard size is 20 rows, 
-    // but on 640x480 it is typically too big.
-    // This takes the height of the screen divides it by two and then subtracts off 
-    // an estimated height of the combobox. I estimate it by taking the max element size
-    // of the drop down and multiplying it by 2 (this is arbitrary) then subtract off
-    // the border and padding of the drop down (again rather arbitrary)
-    // This all breaks down if the font of the combobox is a lot larger then the option items
-    // or CSS style has set the height of the combobox to be rather large.
-    // We can fix these cases later if they actually happen.
-    nsRect screen = nsFormControlFrame::GetUsableScreenRect(aPresContext);
-    nscoord screenHeight = screen.height;
+  // Now compute the height we want to have.
+  // Note: no need to apply min/max constraints, since we have no such
+  // rules applied to the combobox dropdown.
 
-    nscoord availDropHgt = (screenHeight / 2) - (heightOfARow*2); // approx half screen minus combo size
-    availDropHgt -= aReflowState.mComputedBorderPadding.top + aReflowState.mComputedBorderPadding.bottom;
-
-    nscoord hgt = visibleHeight + aReflowState.mComputedBorderPadding.top + aReflowState.mComputedBorderPadding.bottom;
-    if (heightOfARow > 0) {
-      if (hgt > availDropHgt) {
-        visibleHeight = (availDropHgt / heightOfARow) * heightOfARow;
-      }
-      mNumDisplayRows = visibleHeight / heightOfARow;
-    } else {
-      // Hmmm, not sure what to do here. Punt, and make both of them one
-      visibleHeight   = 1;
-      mNumDisplayRows = 1;
-    }
-
-    state.SetComputedHeight(mNumDisplayRows * heightOfARow);
-    // Note: no need to apply min/max constraints, since we have no such
-    // rules applied to the combobox dropdown.
-    // XXXbz this is ending up too big!!  Figure out why.
-  } else if (visibleHeight == 0) {
+  mDropdownCanGrow = false;
+  if (visibleHeight <= 0 || heightOfARow <= 0) {
     // Looks like we have no options.  Just size us to a single row height.
     state.SetComputedHeight(heightOfARow);
+    mNumDisplayRows = 1;
   } else {
-    // Not too big, not too small.  Just use it!
-    state.SetComputedHeight(NS_UNCONSTRAINEDSIZE);
+    nsComboboxControlFrame* combobox = static_cast<nsComboboxControlFrame*>(mComboboxFrame);
+    nsPoint translation;
+    nscoord above, below;
+    combobox->GetAvailableDropdownSpace(&above, &below, &translation);
+    if (above <= 0 && below <= 0) {
+      state.SetComputedHeight(heightOfARow);
+      mNumDisplayRows = 1;
+    } else {
+      nscoord bp = aReflowState.mComputedBorderPadding.TopBottom();
+      nscoord availableHeight = NS_MAX(above, below) - bp;
+      nscoord height = NS_MIN(visibleHeight, availableHeight);
+      PRInt32 rows = height / heightOfARow;
+      mNumDisplayRows = clamped(rows, 1, kMaxDropDownRows);
+      nscoord newHeight = mNumDisplayRows * heightOfARow;
+      state.SetComputedHeight(newHeight);
+      mDropdownCanGrow = visibleHeight - newHeight >= heightOfARow &&
+                         mNumDisplayRows != kMaxDropDownRows;
+    }
   }
 
   // Note: At this point, state.mComputedHeight can be NS_UNCONSTRAINEDSIZE in
@@ -1597,18 +1580,6 @@ nsListControlFrame::GetFormProperty(nsIAtom* aName, nsAString& aValue) const
   return NS_OK;
 }
 
-void
-nsListControlFrame::SyncViewWithFrame()
-{
-    // Resync the view's position with the frame.
-    // The problem is the dropdown's view is attached directly under
-    // the root view. This means its view needs to have its coordinates calculated
-    // as if it were in it's normal position in the view hierarchy.
-  mComboboxFrame->AbsolutelyPositionDropDown();
-
-  nsContainerFrame::PositionFrameView(this);
-}
-
 void
 nsListControlFrame::AboutToDropDown()
 {
@@ -1673,14 +1644,7 @@ nsListControlFrame::DidReflow(nsPresContext*           aPresContext,
   bool wasInterrupted = !mHasPendingInterruptAtStartOfReflow &&
                           aPresContext->HasPendingInterrupt();
 
-  if (IsInDropDownMode()) 
-  {
-    //SyncViewWithFrame();
-    rv = nsHTMLScrollFrame::DidReflow(aPresContext, aReflowState, aStatus);
-    SyncViewWithFrame();
-  } else {
-    rv = nsHTMLScrollFrame::DidReflow(aPresContext, aReflowState, aStatus);
-  }
+  rv = nsHTMLScrollFrame::DidReflow(aPresContext, aReflowState, aStatus);
 
   if (mNeedToReset && !wasInterrupted) {
     mNeedToReset = false;

layout/forms/nsListControlFrame.h

@@ -130,7 +130,6 @@ public:
   virtual void CaptureMouseEvents(bool aGrabMouseEvents);
   virtual nscoord GetHeightOfARow();
   virtual PRInt32 GetNumberOfOptions();  
-  virtual void SyncViewWithFrame();
   virtual void AboutToDropDown();
 
   /**
@@ -232,6 +231,17 @@ public:
    */
   bool IsInDropDownMode() const;
 
+  /**
+   * Return the number of displayed rows in the list.
+   */
+  PRUint32 GetNumDisplayRows() const { return mNumDisplayRows; }
+
+  /**
+   * Return true if the drop-down list can display more rows.
+   * (always false if not in drop-down mode)
+   */
+  bool GetDropdownCanGrow() const { return mDropdownCanGrow; }
+
   /**
    * Dropdowns need views
    */
@@ -414,6 +424,10 @@ protected:
    */
   bool mHasPendingInterruptAtStartOfReflow:1;
 
+  // True if the drop-down can show more rows.  Always false if this list
+  // is not in drop-down mode.
+  bool mDropdownCanGrow:1;
+  
   // The last computed height we reflowed at if we're a combobox dropdown.
   // XXXbz should we be using a subclass here?  Or just not worry
   // about the extra member on listboxes?