Fix for autocomplete widget crashes and misbehavior. This reverts the original fix for bug 192577 since that makes it difficult to reliably determine whether the autocomplete popup is open. Bug 192577 is re-fixed by adding a property to nsIPopupBoxObject to allow the "rollup event" behavior to be specified by an XBL binding. We then set this to "consume" for the urlbar dropdown, so that clicking on the button a second time only dismisses the popup, and skips dispatching the event. Additionally, this change makes us only have a singleton AutoCompleteController for all inputs (this was already the case for HTML inputs, but not XUL inputs). Bug 306067, r+sr=roc.

layout/xul/base/public/nsIPopupBoxObject.idl

@@ -42,7 +42,7 @@
 interface nsIDOMElement;
 
 
-[scriptable, uuid(33C60E14-5150-4876-9A96-2732557E6895)]
+[scriptable, uuid(116ffbea-336d-4ff1-a978-7335f54d11da)]
 interface nsIPopupBoxObject : nsISupports
 {
   void showPopup(in nsIDOMElement srcContent, in nsIDOMElement popupContent,
@@ -67,6 +67,17 @@ interface nsIPopupBoxObject : nsISupports
    */
   void enableRollup(in boolean enableRollup);
 
+  /**
+   * Control whether the event that caused the popup to be automatically
+   * dismissed ("rolled up") should be consumed, or dispatched as a
+   * normal event.  This should be set immediately before calling showPopup()
+   * if non-default behavior is desired.
+   */
+  const PRUint32 ROLLUP_DEFAULT = 0;   /* widget/platform default */
+  const PRUint32 ROLLUP_CONSUME = 1;   /* consume the rollup event */
+  const PRUint32 ROLLUP_NO_CONSUME = 2; /* don't consume the rollup event */
+  void setConsumeRollupEvent(in PRUint32 consume);
+
   /** 
    * Size the popup to the given dimensions
    */

layout/xul/base/src/nsIMenuParent.h

@@ -40,9 +40,9 @@
 #define nsIMenuParent_h___
 
 
-// {0D1F2281-A530-419c-AE2C-21672590A9EC}
+// {33f700c8-976a-4cdb-8f6c-d9f4cfee8366}
 #define NS_IMENUPARENT_IID \
-{ 0x0d1f2281, 0xa530, 0x419c, { 0xae, 0x2c, 0x21, 0x67, 0x25, 0x90, 0xa9, 0xec } }
+{ 0x33f700c8, 0x976a, 0x4cdb, { 0x8f, 0x6c, 0xd9, 0xf4, 0xcf, 0xee, 0x83, 0x66 } }
 
 class nsIMenuFrame;
 class nsIDOMKeyEvent;
@@ -159,6 +159,7 @@ public:
   NS_IMETHOD CancelPendingTimers() = 0;
 
   NS_IMETHOD CreateDismissalListener() = 0;
+  NS_IMETHOD AttachedDismissalListener() = 0;
 
   NS_IMETHOD InstallKeyboardNavigator() = 0;
   NS_IMETHOD RemoveKeyboardNavigator() = 0;

layout/xul/base/src/nsMenuBarFrame.h

@@ -104,6 +104,7 @@ public:
   NS_IMETHOD GetWidget(nsIWidget **aWidget);
   // The dismissal listener gets created and attached to the window.
   NS_IMETHOD CreateDismissalListener();
+  NS_IMETHOD AttachedDismissalListener() { return NS_OK; }
 
   NS_IMETHOD Init(nsPresContext*  aPresContext,
                   nsIContent*      aContent,

layout/xul/base/src/nsMenuDismissalListener.cpp

@@ -39,6 +39,7 @@
 #include "nsMenuDismissalListener.h"
 #include "nsIMenuParent.h"
 #include "nsMenuFrame.h"
+#include "nsIPopupBoxObject.h"
 
 /*
  * nsMenuDismissalListener implementation
@@ -46,7 +47,13 @@
 
 NS_IMPL_ADDREF(nsMenuDismissalListener)
 NS_IMPL_RELEASE(nsMenuDismissalListener)
-NS_IMPL_QUERY_INTERFACE3(nsMenuDismissalListener, nsIDOMMouseListener, nsIMenuRollup, nsIRollupListener)
+NS_INTERFACE_MAP_BEGIN(nsMenuDismissalListener)
+  NS_INTERFACE_MAP_ENTRY(nsIDOMMouseListener)
+  NS_INTERFACE_MAP_ENTRY(nsIDOMEventListener)
+  NS_INTERFACE_MAP_ENTRY(nsIMenuRollup)
+  NS_INTERFACE_MAP_ENTRY(nsIRollupListener)
+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDOMMouseListener)
+NS_INTERFACE_MAP_END
 
 
 ////////////////////////////////////////////////////////////////////////
@@ -99,6 +106,7 @@ nsMenuDismissalListener::SetCurrentMenuParent(nsIMenuParent* aMenuParent)
   mWidget = widget;
 
   NS_ADDREF(nsMenuFrame::sDismissalListener = this);
+  aMenuParent->AttachedDismissalListener();
 }
 
 NS_IMETHODIMP

layout/xul/base/src/nsMenuPopupFrame.cpp

@@ -75,6 +75,7 @@
 #include "nsUnicharUtils.h"
 #include "nsCSSFrameConstructor.h"
 #include "nsIBoxLayout.h"
+#include "nsIPopupBoxObject.h"
 #ifdef XP_WIN
 #include "nsISound.h"
 #endif
@@ -144,7 +145,8 @@ NS_INTERFACE_MAP_END_INHERITING(nsBoxFrame)
 //
 nsMenuPopupFrame::nsMenuPopupFrame(nsIPresShell* aShell)
   :nsBoxFrame(aShell), mCurrentMenu(nsnull), mTimerMenu(nsnull), mCloseTimer(nsnull),
-    mMenuCanOverlapOSBar(PR_FALSE), mShouldAutoPosition(PR_TRUE), mShouldRollup(PR_TRUE)
+   mMenuCanOverlapOSBar(PR_FALSE), mShouldAutoPosition(PR_TRUE), mShouldRollup(PR_TRUE),
+   mConsumeRollupEvent(nsIPopupBoxObject::ROLLUP_DEFAULT)
 {
   SetIsContextMenu(PR_FALSE);   // we're not a context menu by default
 } // ctor
@@ -1256,6 +1258,12 @@ NS_IMETHODIMP nsMenuPopupFrame::ConsumeOutsideClicks(PRBool& aConsumeOutsideClic
    *
    */
 
+  // If the popup has explicitly set a consume mode, honor that.
+  if (mConsumeRollupEvent != nsIPopupBoxObject::ROLLUP_DEFAULT) {
+    aConsumeOutsideClicks = mConsumeRollupEvent == nsIPopupBoxObject::ROLLUP_CONSUME;
+    return NS_OK;
+  }
+
   aConsumeOutsideClicks = PR_TRUE;
 
   nsCOMPtr<nsIContent> parentContent = mContent->GetParent();
@@ -1866,6 +1874,13 @@ nsMenuPopupFrame::CreateDismissalListener()
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsMenuPopupFrame::AttachedDismissalListener()
+{
+  mConsumeRollupEvent = nsIPopupBoxObject::ROLLUP_DEFAULT;
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsMenuPopupFrame::InstallKeyboardNavigator()
 {
@@ -2124,3 +2139,8 @@ nsMenuPopupFrame::EnableRollup(PRBool aShouldRollup)
     CreateDismissalListener();
 }
 
+void
+nsMenuPopupFrame::SetConsumeRollupEvent(PRUint32 aConsumeMode)
+{
+  mConsumeRollupEvent = aConsumeMode;
+}

layout/xul/base/src/nsMenuPopupFrame.h

@@ -115,6 +115,7 @@ public:
 
   // The dismissal listener gets created and attached to the window.
   NS_IMETHOD CreateDismissalListener();
+  NS_IMETHOD AttachedDismissalListener();
 
   // Overridden methods
   NS_IMETHOD Init(nsPresContext*  aPresContext,
@@ -180,6 +181,7 @@ public:
   void GetAutoPosition(PRBool* aShouldAutoPosition);
   void SetAutoPosition(PRBool aShouldAutoPosition);
   void EnableRollup(PRBool aShouldRollup);
+  void SetConsumeRollupEvent(PRUint32 aConsumeMode);
 
   nsIScrollableView* GetScrollableView(nsIFrame* aStart);
   
@@ -224,6 +226,7 @@ protected:
 
   PRPackedBool mShouldAutoPosition; // Should SyncViewWithFrame be allowed to auto position popup?
   PRPackedBool mShouldRollup; // Should this menupopup be allowed to dismiss automatically?
+  PRPackedBool mConsumeRollupEvent; // Should the rollup event be consumed?
 
   nsString     mIncrementalString;  // for incremental typing navigation
 

layout/xul/base/src/nsPopupBoxObject.cpp

@@ -56,76 +56,58 @@
 #include "nsIWidget.h"
 
 
-class nsPopupBoxObject : public nsIPopupBoxObject, public nsBoxObject
+class nsPopupBoxObject : public nsBoxObject,
+                         public nsIPopupBoxObject
 {
 public:
-  NS_DECL_ISUPPORTS
+  NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_NSIPOPUPBOXOBJECT
 
-  nsPopupBoxObject();
+  nsPopupBoxObject() {}
-  virtual ~nsPopupBoxObject();
+protected:
+  virtual ~nsPopupBoxObject() {}
 
+  nsIPopupSetFrame* GetPopupSetFrame();
+  nsMenuPopupFrame* GetMenuPopupFrame()
+  { return NS_STATIC_CAST(nsMenuPopupFrame*, GetFrame()); }
 };
 
-/* Implementation file */
+NS_IMPL_ISUPPORTS_INHERITED1(nsPopupBoxObject, nsBoxObject, nsIPopupBoxObject)
-NS_IMPL_ADDREF(nsPopupBoxObject)
-NS_IMPL_RELEASE(nsPopupBoxObject)
 
-NS_IMETHODIMP 
+nsIPopupSetFrame*
-nsPopupBoxObject::QueryInterface(REFNSIID iid, void** aResult)
+nsPopupBoxObject::GetPopupSetFrame()
 {
-  if (!aResult)
-    return NS_ERROR_NULL_POINTER;
-  
-  if (iid.Equals(NS_GET_IID(nsIPopupBoxObject))) {
-    *aResult = (nsIPopupBoxObject*)this;
-    NS_ADDREF(this);
-    return NS_OK;
-  }
-
-  return nsBoxObject::QueryInterface(iid, aResult);
-}
-  
-nsPopupBoxObject::nsPopupBoxObject()
-{
-}
-
-nsPopupBoxObject::~nsPopupBoxObject()
-{
-  /* destructor code */
-}
-
-/* void openPopup (in boolean openFlag); */
-
-NS_IMETHODIMP
-nsPopupBoxObject::HidePopup()
-{
-  nsIFrame* ourFrame = GetFrame();
-  if (!ourFrame)
-    return NS_OK;
-  
   nsIFrame* rootFrame = mPresShell->FrameManager()->GetRootFrame();
   if (!rootFrame)
-    return NS_OK;
+    return nsnull;
 
   if (rootFrame) {
     rootFrame = rootFrame->GetFirstChild(nsnull);
   }
 
-  nsCOMPtr<nsIRootBox> rootBox(do_QueryInterface(rootFrame));
+  nsIRootBox *rootBox = nsnull;
+  CallQueryInterface(rootFrame, &rootBox);
   if (!rootBox)
-    return NS_OK;
+    return nsnull;
 
   nsIFrame* popupSetFrame = rootBox->GetPopupSetFrame();
   if (!popupSetFrame)
-    return NS_OK;
+    return nsnull;
 
-  nsCOMPtr<nsIPopupSetFrame> popupSet(do_QueryInterface(popupSetFrame));
+  nsIPopupSetFrame *popupSet = nsnull;
-  if (!popupSet)
+  CallQueryInterface(popupSetFrame, &popupSet);
-    return NS_OK;
+  return popupSet;
+}
 
-  popupSet->HidePopup(ourFrame);
+NS_IMETHODIMP
-  popupSet->DestroyPopup(ourFrame, PR_TRUE);
+nsPopupBoxObject::HidePopup()
+{
+  nsIPopupSetFrame *popupSet = GetPopupSetFrame();
+  nsIFrame *ourFrame = GetFrame();
+  if (ourFrame && popupSet) {
+    popupSet->HidePopup(ourFrame);
+    popupSet->DestroyPopup(ourFrame, PR_TRUE);
+  }
 
   return NS_OK;
 }
@@ -134,29 +116,15 @@ NS_IMETHODIMP
 nsPopupBoxObject::ShowPopup(nsIDOMElement* aSrcContent, 
                             nsIDOMElement* aPopupContent, 
                             PRInt32 aXPos, PRInt32 aYPos, 
-                            const PRUnichar *aPopupType, const PRUnichar *anAnchorAlignment, 
+                            const PRUnichar *aPopupType,
+                            const PRUnichar *anAnchorAlignment, 
                             const PRUnichar *aPopupAlignment)
 {
-  nsIFrame* rootFrame = mPresShell->FrameManager()->GetRootFrame();
+  nsIPopupSetFrame *popupSet = GetPopupSetFrame();
-  if (!rootFrame)
+  if (!popupSet) {
     return NS_OK;
-
-  if (rootFrame) {
-    rootFrame = rootFrame->GetFirstChild(nsnull);
   }
 
-  nsCOMPtr<nsIRootBox> rootBox(do_QueryInterface(rootFrame));
-  if (!rootBox)
-    return NS_OK;
-
-  nsIFrame* popupSetFrame = rootBox->GetPopupSetFrame();
-  if (!popupSetFrame)
-    return NS_OK;
-
-  nsCOMPtr<nsIPopupSetFrame> popupSet(do_QueryInterface(popupSetFrame));
-  if (!popupSet)
-    return NS_OK;
-
   nsCOMPtr<nsIContent> srcContent(do_QueryInterface(aSrcContent));
   nsCOMPtr<nsIContent> popupContent(do_QueryInterface(aPopupContent));
 
@@ -189,13 +157,10 @@ nsPopupBoxObject::ShowPopup(nsIDOMElement* aSrcContent,
 NS_IMETHODIMP
 nsPopupBoxObject::MoveTo(PRInt32 aLeft, PRInt32 aTop)
 {
-  nsIFrame* frame = GetFrame();
+  nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame();
-  if (!frame) return NS_OK;
+  if (menuPopupFrame) {
-
+    menuPopupFrame->MoveTo(aLeft, aTop);
-  nsMenuPopupFrame* menuPopupFrame = NS_STATIC_CAST(nsMenuPopupFrame*, frame);
+  }
-  if (!menuPopupFrame) return NS_OK;
-
-  menuPopupFrame->MoveTo(aLeft, aTop);
 
   return NS_OK;
 }
@@ -216,13 +181,10 @@ nsPopupBoxObject::SizeTo(PRInt32 aWidth, PRInt32 aHeight)
 NS_IMETHODIMP
 nsPopupBoxObject::GetAutoPosition(PRBool* aShouldAutoPosition)
 {
-  nsIFrame* frame = GetFrame();
+  nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame();
-  if (!frame) return NS_OK;
+  if (menuPopupFrame) {
-
+    menuPopupFrame->GetAutoPosition(aShouldAutoPosition);
-  nsMenuPopupFrame* menuPopupFrame = NS_STATIC_CAST(nsMenuPopupFrame*, frame);
+  }
-  if (!menuPopupFrame) return NS_OK;
-
-  menuPopupFrame->GetAutoPosition(aShouldAutoPosition);
 
   return NS_OK;
 }
@@ -230,13 +192,10 @@ nsPopupBoxObject::GetAutoPosition(PRBool* aShouldAutoPosition)
 NS_IMETHODIMP
 nsPopupBoxObject::SetAutoPosition(PRBool aShouldAutoPosition)
 {
-  nsIFrame* frame = GetFrame();
+  nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame();
-  if (!frame) return NS_OK;
+  if (menuPopupFrame) {
-
+    menuPopupFrame->SetAutoPosition(aShouldAutoPosition);
-  nsMenuPopupFrame* menuPopupFrame = NS_STATIC_CAST(nsMenuPopupFrame*, frame);
+  }
-  if (!menuPopupFrame) return NS_OK;
-
-  menuPopupFrame->SetAutoPosition(aShouldAutoPosition);
 
   return NS_OK;
 }
@@ -244,13 +203,21 @@ nsPopupBoxObject::SetAutoPosition(PRBool aShouldAutoPosition)
 NS_IMETHODIMP
 nsPopupBoxObject::EnableRollup(PRBool aShouldRollup)
 {
-  nsIFrame* frame = GetFrame();
+  nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame();
-  if (!frame) return NS_OK;
+  if (menuPopupFrame) {
+    menuPopupFrame->EnableRollup(aShouldRollup);
+  }
 
-  nsMenuPopupFrame* menuPopupFrame = NS_STATIC_CAST(nsMenuPopupFrame*, frame);
+  return NS_OK;
-  if (!menuPopupFrame) return NS_OK;
+}
 
-  menuPopupFrame->EnableRollup(aShouldRollup);
+NS_IMETHODIMP
+nsPopupBoxObject::SetConsumeRollupEvent(PRUint32 aConsume)
+{
+  nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame();
+  if (menuPopupFrame) {
+    menuPopupFrame->SetConsumeRollupEvent(aConsume);
+  }
 
   return NS_OK;
 }
@@ -258,17 +225,15 @@ nsPopupBoxObject::EnableRollup(PRBool aShouldRollup)
 NS_IMETHODIMP
 nsPopupBoxObject::EnableKeyboardNavigator(PRBool aEnableKeyboardNavigator)
 {
-  nsIFrame* frame = GetFrame();
+  nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame();
-  if (!frame) return NS_OK;
+  if (menuPopupFrame) {
+    if (aEnableKeyboardNavigator) {
+      menuPopupFrame->InstallKeyboardNavigator();
+    } else {
+      menuPopupFrame->RemoveKeyboardNavigator();
+    }
+  }
 
-  nsMenuPopupFrame* menuPopupFrame = NS_STATIC_CAST(nsMenuPopupFrame*, frame);
-  if (!menuPopupFrame) return NS_OK;
-
-  if (aEnableKeyboardNavigator)
-    menuPopupFrame->InstallKeyboardNavigator();
-  else
-    menuPopupFrame->RemoveKeyboardNavigator();
-  
   return NS_OK;
 }
 

toolkit/components/satchel/src/nsFormFillController.cpp

@@ -98,7 +98,7 @@ nsFormFillController::nsFormFillController() :
   mForceComplete(PR_FALSE),
   mSuppressOnInput(PR_FALSE)
 {
-  mController = do_CreateInstance("@mozilla.org/autocomplete/controller;1");
+  mController = do_GetService("@mozilla.org/autocomplete/controller;1");
 
   mDocShells = do_CreateInstance("@mozilla.org/supports-array;1");
   mPopups = do_CreateInstance("@mozilla.org/supports-array;1");

toolkit/content/widgets/autocomplete.xml

@@ -85,7 +85,7 @@
 
       <constructor><![CDATA[
         mController = Components.classes["@mozilla.org/autocomplete/controller;1"].
-                        createInstance(Components.interfaces.nsIAutoCompleteController);
+                        getService(Components.interfaces.nsIAutoCompleteController);
       ]]></constructor>
 
       <!-- =================== nsIAccessibleProvider =================== -->
@@ -333,6 +333,10 @@
 
       <method name="showHistoryPopup">
         <body><![CDATA[
+          // This is an autocomplete widget, but we want the rollup event
+          // to be consumed, as if we were a menulist.
+          this.popup.popupBoxObject.setConsumeRollupEvent(Components.interfaces.nsIPopupBoxObject.ROLLUP_CONSUME);
+
           this.maxRows = 14;
           this.attachController();
           this.mController.startSearch("");
@@ -724,13 +728,6 @@
         ]]></body>
       </method>
       
-      <method name="clearOpenProperty">
-        <parameter name="aPopupNode"/>
-        <body><![CDATA[
-          aPopupNode.mPopupOpen = false;
-        ]]></body>
-      </method>
-
     </implementation>
 
     <handlers>
@@ -739,7 +736,7 @@
       </handler>
 
       <handler event="popuphiding">
-        setTimeout(this.clearOpenProperty, 0, this);
+        this.mPopupOpen = false;
         this.mInput.maxRows = 6;
       </handler>
     </handlers>
@@ -823,9 +820,7 @@
       <method name="showPopup">
         <body><![CDATA[
           var textbox = document.getBindingParent(this);
-          if (!textbox.popup.mPopupOpen) { 
+          textbox.showHistoryPopup();
-            textbox.showHistoryPopup();
-          }
         ]]></body>
       </method>
     </implementation>