bug 110800 - fire onChange when losing dropdown focus. r=rods@netscape.com, sr=kin@netscape.com

layout/forms/nsComboboxControlFrame.cpp

@@ -306,6 +306,9 @@ nsComboboxControlFrame::nsComboboxControlFrame()
 
   mGoodToGo = PR_FALSE;
 
+  mNeedToFireOnChange = PR_FALSE;
+  mRecentSelectedIndex = -1;
+
   //Shrink the area around it's contents
   //SetFlags(NS_BLOCK_SHRINK_WRAP);
 
@@ -500,12 +503,31 @@ nsComboboxControlFrame::SetFocus(PRBool aOn, PRBool aRepaint)
 {
   if (aOn) {
     mFocused = this;
+
+    // Store up the selected index so when we lose focus we can see if it's
+    // really changed
+    mListControlFrame->GetSelectedIndex(&mRecentSelectedIndex);
   } else {
     mFocused = nsnull;
     if (mDroppedDown) {
       ToggleList(mPresContext);
     }
+
+    // Fire onChange if selected index has changed due to keyboard
+    // (see nsListControlFrame::UpdateSelection)
+    if (mNeedToFireOnChange) {
+      PRInt32 selectedIndex;
+      mListControlFrame->GetSelectedIndex(&selectedIndex);
+      if (selectedIndex != mRecentSelectedIndex) {
+        // mNeedToFireOnChange will be set to false from within FireOnChange
+        mListControlFrame->FireOnChange();
+      } else {
+        // Need to set it to false anyway ... just in case
+        SetNeedToFireOnChange(PR_FALSE);
+      }
+    }
   }
+
   // This is needed on a temporary basis. It causes the focus
   // rect to be drawn. This is much faster than ReResolvingStyle
   // Bug 32920
@@ -2474,6 +2496,23 @@ nsComboboxControlFrame::RollupFromList(nsIPresContext* aPresContext)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsComboboxControlFrame::SetNeedToFireOnChange(PRBool aNeedToFireOnChange)
+{
+  mNeedToFireOnChange = aNeedToFireOnChange;
+  //
+  // If we're setting to false, then that means onChange was fired while
+  // we still may have focus.  We must set recently selected index so that
+  // when we lose focus, we will be able to tell whether the index has changed
+  // since this time.  See SetFocus().
+  //
+  if (!aNeedToFireOnChange) {
+    mListControlFrame->GetSelectedIndex(&mRecentSelectedIndex);
+  }
+
+  return NS_OK;
+}
+
 NS_METHOD 
 nsComboboxControlFrame::Paint(nsIPresContext*     aPresContext,
                              nsIRenderingContext& aRenderingContext,

layout/forms/nsComboboxControlFrame.h

@@ -182,6 +182,7 @@ public:
   NS_IMETHOD GetAbsoluteRect(nsRect* aRect);
   NS_IMETHOD GetIndexOfDisplayArea(PRInt32* aSelectedIndex);
   NS_IMETHOD RedisplaySelectedText();
+  NS_IMETHOD SetNeedToFireOnChange(PRBool aNeedToFireOnChange);
 
   // nsISelectControlFrame
   NS_IMETHOD AddOption(nsIPresContext* aPresContext, PRInt32 index);
@@ -272,7 +273,6 @@ protected:
   nsIPresContext*          mPresContext;             // XXX: Remove the need to cache the pres context.
   nsFormFrame*             mFormFrame;               // Parent Form Frame
   nsCOMPtr<nsITextContent> mDisplayContent;          // Anonymous content used to display the current selection
-  PRPackedBool             mDroppedDown;             // Current state of the dropdown list, PR_TRUE is dropped down
   nsIFrame*                mDisplayFrame;            // frame to display selection
   nsIFrame*                mButtonFrame;             // button frame
   nsIFrame*                mDropdownFrame;           // dropdown list frame
@@ -291,8 +291,11 @@ protected:
   //nscoord               mItemDisplayHeight;
   nsCSSFrameConstructor* mFrameConstructor;
 
+  PRPackedBool          mDroppedDown;             // Current state of the dropdown list, PR_TRUE is dropped down
   PRPackedBool          mGoodToGo;
+  PRPackedBool          mNeedToFireOnChange;
 
+  PRInt32               mRecentSelectedIndex;
   PRInt32               mDisplayedIndex;
 
   // make someone to listen to the button. If its programmatically pressed by someone like Accessibility

layout/forms/nsIComboboxControlFrame.h

@@ -66,31 +66,26 @@ public:
 
   /**
    * Indicates whether the list is dropped down
-   *
    */
   NS_IMETHOD IsDroppedDown(PRBool * aDoDropDown) = 0;
 
   /**
    * Shows or hides the drop down
-   *
    */
   NS_IMETHOD ShowDropDown(PRBool aDoDropDown) = 0;
 
   /**
    * Gets the Drop Down List
-   *
    */
   NS_IMETHOD GetDropDown(nsIFrame** aDropDownFrame) = 0;
 
   /**
    * Sets the Drop Down List
-   *
    */
   NS_IMETHOD SetDropDown(nsIFrame* aDropDownFrame) = 0;
 
   /**
    * Tells the combobox to roll up
-   *
    */
   NS_IMETHOD RollupFromList(nsIPresContext* aPresContext) = 0;
 
@@ -99,6 +94,11 @@ public:
    */
   NS_IMETHOD RedisplaySelectedText() = 0;
 
+  /**
+   * Method for the listbox to notify the combobox that onChange has been fired
+   */
+  NS_IMETHOD SetNeedToFireOnChange(PRBool aNeedToFireOnChange) = 0;
+
   /**
    *
    */

layout/forms/nsIListControlFrame.h

@@ -100,19 +100,19 @@ public:
   NS_IMETHOD SyncViewWithFrame(nsIPresContext* aPresContext) = 0;
 
   /**
-   * 
+   * Called by combobox when it's about to drop down
    */
   NS_IMETHOD AboutToDropDown() = 0;
 
   /**
-   * 
+   * Called by combobox when it's about to roll up
    */
   NS_IMETHOD AboutToRollup() = 0;
 
   /**
-   *
+   * Fire on change (used by combobox)
    */
-  NS_IMETHOD UpdateSelection() = 0;
+  NS_IMETHOD FireOnChange() = 0;
 
   /**
    *

layout/forms/nsListControlFrame.cpp

@@ -2093,17 +2093,18 @@ nsListControlFrame::ToggleOptionSelectedFromFrame(PRInt32 aIndex)
 NS_IMETHODIMP
 nsListControlFrame::UpdateSelection()
 {
-  if (!mIsAllFramesHere || !mIsAllContentHere) {
-    return NS_OK;
-  }
   nsresult rv = NS_OK;
 
-  PRBool isDroppedDown = PR_FALSE;
+  if (mIsAllFramesHere) {
-  if (mComboboxFrame != nsnull) {
+    // if it's a combobox, display the new text
-    mComboboxFrame->IsDroppedDown(&isDroppedDown);
+    if (mComboboxFrame) {
-  }
+      rv = mComboboxFrame->RedisplaySelectedText();
-  if (!isDroppedDown) {
+      rv = mComboboxFrame->SetNeedToFireOnChange(PR_TRUE);
-    rv = FireOnChange(); // Dispatch event
+    }
+    // if it's a listbox, fire on change
+    else if (mIsAllContentHere) {
+      rv = FireOnChange();
+    }
   }
 
   return rv;
@@ -2123,6 +2124,7 @@ nsListControlFrame::ComboboxFinish(PRInt32 aIndex)
     }
 
     mComboboxFrame->RollupFromList(mPresContext);
+
     if (aIndex != mSelectedIndexWhenPoppedDown) {
       FireOnChange();
     }
@@ -2137,10 +2139,10 @@ nsListControlFrame::GetOptionsContainer(nsIPresContext* aPresContext,
 }
 
 // Send out an onchange notification.
-nsresult
+NS_IMETHODIMP
 nsListControlFrame::FireOnChange()
 {
-  nsresult ret = NS_ERROR_FAILURE;
+  nsresult rv = NS_OK;
 
   // Dispatch the NS_FORM_CHANGE event
   nsEventStatus status = nsEventStatus_eIgnore;
@@ -2149,12 +2151,17 @@ nsListControlFrame::FireOnChange()
   event.message = NS_FORM_CHANGE;
 
   nsCOMPtr<nsIPresShell> presShell;
-  mPresContext->GetShell(getter_AddRefs(presShell));
+  rv = mPresContext->GetShell(getter_AddRefs(presShell));
   if (presShell) {
-    ret = presShell->HandleEventWithTarget(&event, this, nsnull, NS_EVENT_FLAG_INIT, &status);
+    rv = presShell->HandleEventWithTarget(&event, this, nsnull,
+                                           NS_EVENT_FLAG_INIT, &status);
+    // Obviously the combobox doesn't need to fire onChange anymore
+    if (NS_SUCCEEDED(rv) && mComboboxFrame) {
+      rv = mComboboxFrame->SetNeedToFireOnChange(PR_FALSE);
+    }
   }
 
-  return ret;
+  return rv;
 }
 
 //---------------------------------------------------------
@@ -2265,7 +2272,6 @@ nsListControlFrame::GetViewOffset(nsIViewManager* aManager, nsIView* aView,
   aPoint.y = 0;
  
   nsIView *parent;
-  nsRect bounds;
 
   parent = aView;
   while (nsnull != parent) {
@@ -2614,7 +2620,7 @@ nsListControlFrame::MouseUp(nsIDOMEvent* aMouseEvent)
     CaptureMouseEvents(mPresContext, PR_FALSE);
     // Notify
     if (mChangesSinceDragStart) {
-      UpdateSelection();
+      FireOnChange();
     }
 #if 0 // XXX - this is a partial fix for Bug 29990
     if (mSelectedIndex != mStartExtendedIndex) {
@@ -3228,13 +3234,8 @@ nsListControlFrame::KeyPress(nsIDOMEvent* aKeyEvent)
             if (firstChar == (PRUnichar)code) {
               PRBool wasChanged = PerformSelection(selectedIndex,
                                                    isShift, isControl);
-              // If it's a combobox, redisplay the text
-              if (mComboboxFrame && mIsAllFramesHere) {
-                mComboboxFrame->RedisplaySelectedText();
-              }
-              // Fire the event (unless it's a dropped down combobox)
               if (wasChanged) {
-                UpdateSelection(); // dispatch event
+                UpdateSelection(); // dispatch event, update combobox, etc.
               }
               break;
             }
@@ -3260,13 +3261,8 @@ nsListControlFrame::KeyPress(nsIDOMEvent* aKeyEvent)
       ScrollToIndex(newIndex);
     } else {
       PRBool wasChanged = PerformSelection(newIndex, isShift, isControl);
-      // If combobox, redisplay selected text.
-      if (mComboboxFrame && mIsAllFramesHere) {
-        mComboboxFrame->RedisplaySelectedText();
-      }
-      // Fire the event (unless it's a dropped down combobox)
       if (wasChanged) {
-        UpdateSelection();
+        UpdateSelection(); // dispatch event, update combobox, etc.
       }
     }
 

layout/forms/nsListControlFrame.h

@@ -276,6 +276,8 @@ public:
   NS_IMETHOD UpdateSelection();
   NS_IMETHOD SetOverrideReflowOptimization(PRBool aValue) { mOverrideReflowOpt = aValue; return NS_OK; }
   NS_IMETHOD GetOptionsContainer(nsIPresContext* aPresContext, nsIFrame** aFrame);
+  NS_IMETHOD FireOnChange();
+  
 
   // nsISelectControlFrame
   NS_IMETHOD AddOption(nsIPresContext* aPresContext, PRInt32 index);
@@ -383,9 +385,6 @@ protected:
   void     StopUpdateTimer();
   void     ItemsHaveBeenRemoved(nsIPresContext * aPresContext);
 
-  // fire onChange
-  nsresult FireOnChange();
-  
   // Data Members
   nsFormFrame* mFormFrame;
 

layout/html/forms/public/nsIComboboxControlFrame.h

@@ -66,31 +66,26 @@ public:
 
   /**
    * Indicates whether the list is dropped down
-   *
    */
   NS_IMETHOD IsDroppedDown(PRBool * aDoDropDown) = 0;
 
   /**
    * Shows or hides the drop down
-   *
    */
   NS_IMETHOD ShowDropDown(PRBool aDoDropDown) = 0;
 
   /**
    * Gets the Drop Down List
-   *
    */
   NS_IMETHOD GetDropDown(nsIFrame** aDropDownFrame) = 0;
 
   /**
    * Sets the Drop Down List
-   *
    */
   NS_IMETHOD SetDropDown(nsIFrame* aDropDownFrame) = 0;
 
   /**
    * Tells the combobox to roll up
-   *
    */
   NS_IMETHOD RollupFromList(nsIPresContext* aPresContext) = 0;
 
@@ -99,6 +94,11 @@ public:
    */
   NS_IMETHOD RedisplaySelectedText() = 0;
 
+  /**
+   * Method for the listbox to notify the combobox that onChange has been fired
+   */
+  NS_IMETHOD SetNeedToFireOnChange(PRBool aNeedToFireOnChange) = 0;
+
   /**
    *
    */

layout/html/forms/public/nsIListControlFrame.h

@@ -100,19 +100,19 @@ public:
   NS_IMETHOD SyncViewWithFrame(nsIPresContext* aPresContext) = 0;
 
   /**
-   * 
+   * Called by combobox when it's about to drop down
    */
   NS_IMETHOD AboutToDropDown() = 0;
 
   /**
-   * 
+   * Called by combobox when it's about to roll up
    */
   NS_IMETHOD AboutToRollup() = 0;
 
   /**
-   *
+   * Fire on change (used by combobox)
    */
-  NS_IMETHOD UpdateSelection() = 0;
+  NS_IMETHOD FireOnChange() = 0;
 
   /**
    *

layout/html/forms/src/nsComboboxControlFrame.cpp

@@ -306,6 +306,9 @@ nsComboboxControlFrame::nsComboboxControlFrame()
 
   mGoodToGo = PR_FALSE;
 
+  mNeedToFireOnChange = PR_FALSE;
+  mRecentSelectedIndex = -1;
+
   //Shrink the area around it's contents
   //SetFlags(NS_BLOCK_SHRINK_WRAP);
 
@@ -500,12 +503,31 @@ nsComboboxControlFrame::SetFocus(PRBool aOn, PRBool aRepaint)
 {
   if (aOn) {
     mFocused = this;
+
+    // Store up the selected index so when we lose focus we can see if it's
+    // really changed
+    mListControlFrame->GetSelectedIndex(&mRecentSelectedIndex);
   } else {
     mFocused = nsnull;
     if (mDroppedDown) {
       ToggleList(mPresContext);
     }
+
+    // Fire onChange if selected index has changed due to keyboard
+    // (see nsListControlFrame::UpdateSelection)
+    if (mNeedToFireOnChange) {
+      PRInt32 selectedIndex;
+      mListControlFrame->GetSelectedIndex(&selectedIndex);
+      if (selectedIndex != mRecentSelectedIndex) {
+        // mNeedToFireOnChange will be set to false from within FireOnChange
+        mListControlFrame->FireOnChange();
+      } else {
+        // Need to set it to false anyway ... just in case
+        SetNeedToFireOnChange(PR_FALSE);
+      }
+    }
   }
+
   // This is needed on a temporary basis. It causes the focus
   // rect to be drawn. This is much faster than ReResolvingStyle
   // Bug 32920
@@ -2474,6 +2496,23 @@ nsComboboxControlFrame::RollupFromList(nsIPresContext* aPresContext)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsComboboxControlFrame::SetNeedToFireOnChange(PRBool aNeedToFireOnChange)
+{
+  mNeedToFireOnChange = aNeedToFireOnChange;
+  //
+  // If we're setting to false, then that means onChange was fired while
+  // we still may have focus.  We must set recently selected index so that
+  // when we lose focus, we will be able to tell whether the index has changed
+  // since this time.  See SetFocus().
+  //
+  if (!aNeedToFireOnChange) {
+    mListControlFrame->GetSelectedIndex(&mRecentSelectedIndex);
+  }
+
+  return NS_OK;
+}
+
 NS_METHOD 
 nsComboboxControlFrame::Paint(nsIPresContext*     aPresContext,
                              nsIRenderingContext& aRenderingContext,

layout/html/forms/src/nsComboboxControlFrame.h

@@ -182,6 +182,7 @@ public:
   NS_IMETHOD GetAbsoluteRect(nsRect* aRect);
   NS_IMETHOD GetIndexOfDisplayArea(PRInt32* aSelectedIndex);
   NS_IMETHOD RedisplaySelectedText();
+  NS_IMETHOD SetNeedToFireOnChange(PRBool aNeedToFireOnChange);
 
   // nsISelectControlFrame
   NS_IMETHOD AddOption(nsIPresContext* aPresContext, PRInt32 index);
@@ -272,7 +273,6 @@ protected:
   nsIPresContext*          mPresContext;             // XXX: Remove the need to cache the pres context.
   nsFormFrame*             mFormFrame;               // Parent Form Frame
   nsCOMPtr<nsITextContent> mDisplayContent;          // Anonymous content used to display the current selection
-  PRPackedBool             mDroppedDown;             // Current state of the dropdown list, PR_TRUE is dropped down
   nsIFrame*                mDisplayFrame;            // frame to display selection
   nsIFrame*                mButtonFrame;             // button frame
   nsIFrame*                mDropdownFrame;           // dropdown list frame
@@ -291,8 +291,11 @@ protected:
   //nscoord               mItemDisplayHeight;
   nsCSSFrameConstructor* mFrameConstructor;
 
+  PRPackedBool          mDroppedDown;             // Current state of the dropdown list, PR_TRUE is dropped down
   PRPackedBool          mGoodToGo;
+  PRPackedBool          mNeedToFireOnChange;
 
+  PRInt32               mRecentSelectedIndex;
   PRInt32               mDisplayedIndex;
 
   // make someone to listen to the button. If its programmatically pressed by someone like Accessibility

layout/html/forms/src/nsListControlFrame.cpp

@@ -2093,17 +2093,18 @@ nsListControlFrame::ToggleOptionSelectedFromFrame(PRInt32 aIndex)
 NS_IMETHODIMP
 nsListControlFrame::UpdateSelection()
 {
-  if (!mIsAllFramesHere || !mIsAllContentHere) {
-    return NS_OK;
-  }
   nsresult rv = NS_OK;
 
-  PRBool isDroppedDown = PR_FALSE;
+  if (mIsAllFramesHere) {
-  if (mComboboxFrame != nsnull) {
+    // if it's a combobox, display the new text
-    mComboboxFrame->IsDroppedDown(&isDroppedDown);
+    if (mComboboxFrame) {
-  }
+      rv = mComboboxFrame->RedisplaySelectedText();
-  if (!isDroppedDown) {
+      rv = mComboboxFrame->SetNeedToFireOnChange(PR_TRUE);
-    rv = FireOnChange(); // Dispatch event
+    }
+    // if it's a listbox, fire on change
+    else if (mIsAllContentHere) {
+      rv = FireOnChange();
+    }
   }
 
   return rv;
@@ -2123,6 +2124,7 @@ nsListControlFrame::ComboboxFinish(PRInt32 aIndex)
     }
 
     mComboboxFrame->RollupFromList(mPresContext);
+
     if (aIndex != mSelectedIndexWhenPoppedDown) {
       FireOnChange();
     }
@@ -2137,10 +2139,10 @@ nsListControlFrame::GetOptionsContainer(nsIPresContext* aPresContext,
 }
 
 // Send out an onchange notification.
-nsresult
+NS_IMETHODIMP
 nsListControlFrame::FireOnChange()
 {
-  nsresult ret = NS_ERROR_FAILURE;
+  nsresult rv = NS_OK;
 
   // Dispatch the NS_FORM_CHANGE event
   nsEventStatus status = nsEventStatus_eIgnore;
@@ -2149,12 +2151,17 @@ nsListControlFrame::FireOnChange()
   event.message = NS_FORM_CHANGE;
 
   nsCOMPtr<nsIPresShell> presShell;
-  mPresContext->GetShell(getter_AddRefs(presShell));
+  rv = mPresContext->GetShell(getter_AddRefs(presShell));
   if (presShell) {
-    ret = presShell->HandleEventWithTarget(&event, this, nsnull, NS_EVENT_FLAG_INIT, &status);
+    rv = presShell->HandleEventWithTarget(&event, this, nsnull,
+                                           NS_EVENT_FLAG_INIT, &status);
+    // Obviously the combobox doesn't need to fire onChange anymore
+    if (NS_SUCCEEDED(rv) && mComboboxFrame) {
+      rv = mComboboxFrame->SetNeedToFireOnChange(PR_FALSE);
+    }
   }
 
-  return ret;
+  return rv;
 }
 
 //---------------------------------------------------------
@@ -2265,7 +2272,6 @@ nsListControlFrame::GetViewOffset(nsIViewManager* aManager, nsIView* aView,
   aPoint.y = 0;
  
   nsIView *parent;
-  nsRect bounds;
 
   parent = aView;
   while (nsnull != parent) {
@@ -2614,7 +2620,7 @@ nsListControlFrame::MouseUp(nsIDOMEvent* aMouseEvent)
     CaptureMouseEvents(mPresContext, PR_FALSE);
     // Notify
     if (mChangesSinceDragStart) {
-      UpdateSelection();
+      FireOnChange();
     }
 #if 0 // XXX - this is a partial fix for Bug 29990
     if (mSelectedIndex != mStartExtendedIndex) {
@@ -3228,13 +3234,8 @@ nsListControlFrame::KeyPress(nsIDOMEvent* aKeyEvent)
             if (firstChar == (PRUnichar)code) {
               PRBool wasChanged = PerformSelection(selectedIndex,
                                                    isShift, isControl);
-              // If it's a combobox, redisplay the text
-              if (mComboboxFrame && mIsAllFramesHere) {
-                mComboboxFrame->RedisplaySelectedText();
-              }
-              // Fire the event (unless it's a dropped down combobox)
               if (wasChanged) {
-                UpdateSelection(); // dispatch event
+                UpdateSelection(); // dispatch event, update combobox, etc.
               }
               break;
             }
@@ -3260,13 +3261,8 @@ nsListControlFrame::KeyPress(nsIDOMEvent* aKeyEvent)
       ScrollToIndex(newIndex);
     } else {
       PRBool wasChanged = PerformSelection(newIndex, isShift, isControl);
-      // If combobox, redisplay selected text.
-      if (mComboboxFrame && mIsAllFramesHere) {
-        mComboboxFrame->RedisplaySelectedText();
-      }
-      // Fire the event (unless it's a dropped down combobox)
       if (wasChanged) {
-        UpdateSelection();
+        UpdateSelection(); // dispatch event, update combobox, etc.
       }
     }
 

layout/html/forms/src/nsListControlFrame.h

@@ -276,6 +276,8 @@ public:
   NS_IMETHOD UpdateSelection();
   NS_IMETHOD SetOverrideReflowOptimization(PRBool aValue) { mOverrideReflowOpt = aValue; return NS_OK; }
   NS_IMETHOD GetOptionsContainer(nsIPresContext* aPresContext, nsIFrame** aFrame);
+  NS_IMETHOD FireOnChange();
+  
 
   // nsISelectControlFrame
   NS_IMETHOD AddOption(nsIPresContext* aPresContext, PRInt32 index);
@@ -383,9 +385,6 @@ protected:
   void     StopUpdateTimer();
   void     ItemsHaveBeenRemoved(nsIPresContext * aPresContext);
 
-  // fire onChange
-  nsresult FireOnChange();
-  
   // Data Members
   nsFormFrame* mFormFrame;