From 6f179597968c8b9e2f5522d3156bf2c5d0d79fad Mon Sep 17 00:00:00 2001 From: "bryner%brianryner.com" Date: Sat, 31 Dec 2005 23:34:46 +0000 Subject: [PATCH] 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 | 13 +- layout/xul/base/src/nsIMenuParent.h | 5 +- layout/xul/base/src/nsMenuBarFrame.h | 1 + .../xul/base/src/nsMenuDismissalListener.cpp | 10 +- layout/xul/base/src/nsMenuPopupFrame.cpp | 22 ++- layout/xul/base/src/nsMenuPopupFrame.h | 3 + layout/xul/base/src/nsPopupBoxObject.cpp | 169 +++++++----------- .../satchel/src/nsFormFillController.cpp | 2 +- toolkit/content/widgets/autocomplete.xml | 19 +- 9 files changed, 124 insertions(+), 120 deletions(-) diff --git a/layout/xul/base/public/nsIPopupBoxObject.idl b/layout/xul/base/public/nsIPopupBoxObject.idl index 57b5c51c5487..02344365626a 100644 --- a/layout/xul/base/public/nsIPopupBoxObject.idl +++ b/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 */ diff --git a/layout/xul/base/src/nsIMenuParent.h b/layout/xul/base/src/nsIMenuParent.h index 70e04dd3894a..7b366c0c84ee 100644 --- a/layout/xul/base/src/nsIMenuParent.h +++ b/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; diff --git a/layout/xul/base/src/nsMenuBarFrame.h b/layout/xul/base/src/nsMenuBarFrame.h index 163f1d5848a6..156bc28287ca 100644 --- a/layout/xul/base/src/nsMenuBarFrame.h +++ b/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, diff --git a/layout/xul/base/src/nsMenuDismissalListener.cpp b/layout/xul/base/src/nsMenuDismissalListener.cpp index faf20ec4a177..266a612adf83 100644 --- a/layout/xul/base/src/nsMenuDismissalListener.cpp +++ b/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 diff --git a/layout/xul/base/src/nsMenuPopupFrame.cpp b/layout/xul/base/src/nsMenuPopupFrame.cpp index 90ad0d0716cd..5d0437eeb322 100644 --- a/layout/xul/base/src/nsMenuPopupFrame.cpp +++ b/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 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; +} diff --git a/layout/xul/base/src/nsMenuPopupFrame.h b/layout/xul/base/src/nsMenuPopupFrame.h index 39500195dde7..d4800770f6de 100644 --- a/layout/xul/base/src/nsMenuPopupFrame.h +++ b/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 diff --git a/layout/xul/base/src/nsPopupBoxObject.cpp b/layout/xul/base/src/nsPopupBoxObject.cpp index 72f1ced26611..ca539f225146 100644 --- a/layout/xul/base/src/nsPopupBoxObject.cpp +++ b/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(); - virtual ~nsPopupBoxObject(); + nsPopupBoxObject() {} +protected: + virtual ~nsPopupBoxObject() {} + nsIPopupSetFrame* GetPopupSetFrame(); + nsMenuPopupFrame* GetMenuPopupFrame() + { return NS_STATIC_CAST(nsMenuPopupFrame*, GetFrame()); } }; -/* Implementation file */ -NS_IMPL_ADDREF(nsPopupBoxObject) -NS_IMPL_RELEASE(nsPopupBoxObject) +NS_IMPL_ISUPPORTS_INHERITED1(nsPopupBoxObject, nsBoxObject, nsIPopupBoxObject) -NS_IMETHODIMP -nsPopupBoxObject::QueryInterface(REFNSIID iid, void** aResult) +nsIPopupSetFrame* +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 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 popupSet(do_QueryInterface(popupSetFrame)); - if (!popupSet) - return NS_OK; + nsIPopupSetFrame *popupSet = nsnull; + CallQueryInterface(popupSetFrame, &popupSet); + return popupSet; +} - popupSet->HidePopup(ourFrame); - popupSet->DestroyPopup(ourFrame, PR_TRUE); +NS_IMETHODIMP +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(); - if (!rootFrame) + nsIPopupSetFrame *popupSet = GetPopupSetFrame(); + if (!popupSet) { return NS_OK; - - if (rootFrame) { - rootFrame = rootFrame->GetFirstChild(nsnull); } - nsCOMPtr rootBox(do_QueryInterface(rootFrame)); - if (!rootBox) - return NS_OK; - - nsIFrame* popupSetFrame = rootBox->GetPopupSetFrame(); - if (!popupSetFrame) - return NS_OK; - - nsCOMPtr popupSet(do_QueryInterface(popupSetFrame)); - if (!popupSet) - return NS_OK; - nsCOMPtr srcContent(do_QueryInterface(aSrcContent)); nsCOMPtr popupContent(do_QueryInterface(aPopupContent)); @@ -189,13 +157,10 @@ nsPopupBoxObject::ShowPopup(nsIDOMElement* aSrcContent, NS_IMETHODIMP nsPopupBoxObject::MoveTo(PRInt32 aLeft, PRInt32 aTop) { - nsIFrame* frame = GetFrame(); - if (!frame) return NS_OK; - - nsMenuPopupFrame* menuPopupFrame = NS_STATIC_CAST(nsMenuPopupFrame*, frame); - if (!menuPopupFrame) return NS_OK; - - menuPopupFrame->MoveTo(aLeft, aTop); + nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame(); + if (menuPopupFrame) { + 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(); - if (!frame) return NS_OK; - - nsMenuPopupFrame* menuPopupFrame = NS_STATIC_CAST(nsMenuPopupFrame*, frame); - if (!menuPopupFrame) return NS_OK; - - menuPopupFrame->GetAutoPosition(aShouldAutoPosition); + nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame(); + if (menuPopupFrame) { + menuPopupFrame->GetAutoPosition(aShouldAutoPosition); + } return NS_OK; } @@ -230,13 +192,10 @@ nsPopupBoxObject::GetAutoPosition(PRBool* aShouldAutoPosition) NS_IMETHODIMP nsPopupBoxObject::SetAutoPosition(PRBool aShouldAutoPosition) { - nsIFrame* frame = GetFrame(); - if (!frame) return NS_OK; - - nsMenuPopupFrame* menuPopupFrame = NS_STATIC_CAST(nsMenuPopupFrame*, frame); - if (!menuPopupFrame) return NS_OK; - - menuPopupFrame->SetAutoPosition(aShouldAutoPosition); + nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame(); + if (menuPopupFrame) { + menuPopupFrame->SetAutoPosition(aShouldAutoPosition); + } return NS_OK; } @@ -244,13 +203,21 @@ nsPopupBoxObject::SetAutoPosition(PRBool aShouldAutoPosition) NS_IMETHODIMP nsPopupBoxObject::EnableRollup(PRBool aShouldRollup) { - nsIFrame* frame = GetFrame(); - if (!frame) return NS_OK; + nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame(); + if (menuPopupFrame) { + menuPopupFrame->EnableRollup(aShouldRollup); + } - nsMenuPopupFrame* menuPopupFrame = NS_STATIC_CAST(nsMenuPopupFrame*, frame); - if (!menuPopupFrame) return NS_OK; + 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(); - if (!frame) return NS_OK; + nsMenuPopupFrame *menuPopupFrame = GetMenuPopupFrame(); + 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; } diff --git a/toolkit/components/satchel/src/nsFormFillController.cpp b/toolkit/components/satchel/src/nsFormFillController.cpp index 559c152f6d21..0d115f0e1e1a 100644 --- a/toolkit/components/satchel/src/nsFormFillController.cpp +++ b/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"); diff --git a/toolkit/content/widgets/autocomplete.xml b/toolkit/content/widgets/autocomplete.xml index c09c5cb8dbf0..5138503dc1c8 100644 --- a/toolkit/content/widgets/autocomplete.xml +++ b/toolkit/content/widgets/autocomplete.xml @@ -85,7 +85,7 @@ @@ -333,6 +333,10 @@ - - - - - @@ -739,7 +736,7 @@ - setTimeout(this.clearOpenProperty, 0, this); + this.mPopupOpen = false; this.mInput.maxRows = 6; @@ -823,9 +820,7 @@