From 58213a9d14a78980960ac342c49b21a7f1d76fe4 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Tue, 16 Oct 2012 15:37:26 +0300 Subject: [PATCH] Bug 801609 - Simplify popup and contextmenu handling and make the listener skippable, r=mccr8 --HG-- rename : content/canvas/test/test_toBlob.html => content/canvas/test/test_mozGetAsFile.html rename : dom/base/test/test_gsp-qualified.html => dom/base/test/test_gsp-standards.html extra : rebase_source : 8e017d2fa6ff12cfcf0b0f8a56dadd541aca16bf --- content/base/src/FragmentOrElement.cpp | 10 --- content/base/src/nsGkAtomList.h | 2 - content/xul/content/src/nsXULElement.cpp | 46 ++---------- content/xul/content/src/nsXULElement.h | 8 +- .../xul/content/src/nsXULPopupListener.cpp | 74 +++++++++++-------- content/xul/content/src/nsXULPopupListener.h | 8 +- 6 files changed, 58 insertions(+), 90 deletions(-) diff --git a/content/base/src/FragmentOrElement.cpp b/content/base/src/FragmentOrElement.cpp index bc2ec9256850..5f6e888613aa 100644 --- a/content/base/src/FragmentOrElement.cpp +++ b/content/base/src/FragmentOrElement.cpp @@ -1165,9 +1165,6 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(FragmentOrElement) tmp->DeleteProperty(nsGkAtoms::itemtype); tmp->DeleteProperty(nsGkAtoms::itemref); tmp->DeleteProperty(nsGkAtoms::itemprop); - } else if (tmp->IsXUL()) { - tmp->DeleteProperty(nsGkAtoms::contextmenulistener); - tmp->DeleteProperty(nsGkAtoms::popuplistener); } } @@ -1708,13 +1705,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(FragmentOrElement) cb.NoteXPCOMChild(property); property = static_cast(tmp->GetProperty(nsGkAtoms::itemtype)); cb.NoteXPCOMChild(property); - } else if (tmp->IsXUL()) { - nsISupports* property = static_cast - (tmp->GetProperty(nsGkAtoms::contextmenulistener)); - cb.NoteXPCOMChild(property); - property = static_cast - (tmp->GetProperty(nsGkAtoms::popuplistener)); - cb.NoteXPCOMChild(property); } } diff --git a/content/base/src/nsGkAtomList.h b/content/base/src/nsGkAtomList.h index 2c6ecf8cb93a..9cf9d22e2b3c 100644 --- a/content/base/src/nsGkAtomList.h +++ b/content/base/src/nsGkAtomList.h @@ -226,7 +226,6 @@ GK_ATOM(headerContentStyleType, "content-style-type") GK_ATOM(headerContentType, "content-type") GK_ATOM(context, "context") GK_ATOM(contextmenu, "contextmenu") -GK_ATOM(contextmenulistener, "contextmenulistener") GK_ATOM(control, "control") GK_ATOM(controls, "controls") GK_ATOM(coords, "coords") @@ -833,7 +832,6 @@ GK_ATOM(popupanchor, "popupanchor") GK_ATOM(popupgroup, "popupgroup") GK_ATOM(popuphidden, "popuphidden") GK_ATOM(popuphiding, "popuphiding") -GK_ATOM(popuplistener, "popuplistener") GK_ATOM(popupset, "popupset") GK_ATOM(popupshowing, "popupshowing") GK_ATOM(popupshown, "popupshown") diff --git a/content/xul/content/src/nsXULElement.cpp b/content/xul/content/src/nsXULElement.cpp index 83c866451816..20c33548a62a 100644 --- a/content/xul/content/src/nsXULElement.cpp +++ b/content/xul/content/src/nsXULElement.cpp @@ -1554,58 +1554,26 @@ nsXULElement::IsNodeOfType(uint32_t aFlags) const return !(aFlags & ~eCONTENT); } -static void -PopupListenerPropertyDtor(void* aObject, nsIAtom* aPropertyName, - void* aPropertyValue, void* aData) -{ - nsIDOMEventListener* listener = - static_cast(aPropertyValue); - if (!listener) { - return; - } - nsEventListenerManager* manager = static_cast(aObject)-> - GetListenerManager(false); - if (manager) { - manager->RemoveEventListenerByType(listener, - NS_LITERAL_STRING("mousedown"), - NS_EVENT_FLAG_BUBBLE | - NS_EVENT_FLAG_SYSTEM_EVENT); - manager->RemoveEventListenerByType(listener, - NS_LITERAL_STRING("contextmenu"), - NS_EVENT_FLAG_BUBBLE | - NS_EVENT_FLAG_SYSTEM_EVENT); - } - NS_RELEASE(listener); -} - nsresult nsXULElement::AddPopupListener(nsIAtom* aName) { // Add a popup listener to the element bool isContext = (aName == nsGkAtoms::context || aName == nsGkAtoms::contextmenu); - nsIAtom* listenerAtom = isContext ? - nsGkAtoms::contextmenulistener : - nsGkAtoms::popuplistener; + uint32_t listenerFlag = isContext ? + XUL_ELEMENT_HAS_CONTENTMENU_LISTENER : + XUL_ELEMENT_HAS_POPUP_LISTENER; - nsCOMPtr popupListener = - static_cast(GetProperty(listenerAtom)); - if (popupListener) { - // Popup listener is already installed. + if (HasFlag(listenerFlag)) { return NS_OK; } - popupListener = new nsXULPopupListener(this, isContext); + nsCOMPtr listener = + new nsXULPopupListener(this, isContext); // Add the popup as a listener on this element. nsEventListenerManager* manager = GetListenerManager(true); - NS_ENSURE_TRUE(manager, NS_ERROR_FAILURE); - nsresult rv = SetProperty(listenerAtom, popupListener, - PopupListenerPropertyDtor, true); - NS_ENSURE_SUCCESS(rv, rv); - // Want the property to have a reference to the listener. - nsIDOMEventListener* listener = nullptr; - popupListener.swap(listener); + SetFlags(listenerFlag); if (isContext) { manager->AddEventListenerByType(listener, diff --git a/content/xul/content/src/nsXULElement.h b/content/xul/content/src/nsXULElement.h index 098e67ba6257..53aa453cc5f0 100644 --- a/content/xul/content/src/nsXULElement.h +++ b/content/xul/content/src/nsXULElement.h @@ -321,11 +321,13 @@ public: // XUL element specific bits enum { - XUL_ELEMENT_TEMPLATE_GENERATED = XUL_ELEMENT_FLAG_BIT(0) + XUL_ELEMENT_TEMPLATE_GENERATED = XUL_ELEMENT_FLAG_BIT(0), + XUL_ELEMENT_HAS_CONTENTMENU_LISTENER = XUL_ELEMENT_FLAG_BIT(1), + XUL_ELEMENT_HAS_POPUP_LISTENER = XUL_ELEMENT_FLAG_BIT(2) }; -// Make sure we have space for our bit -PR_STATIC_ASSERT(ELEMENT_TYPE_SPECIFIC_BITS_OFFSET < 32); +// Make sure we have space for our bits +PR_STATIC_ASSERT((ELEMENT_TYPE_SPECIFIC_BITS_OFFSET + 2) < 32); #undef XUL_ELEMENT_FLAG_BIT diff --git a/content/xul/content/src/nsXULPopupListener.cpp b/content/xul/content/src/nsXULPopupListener.cpp index d4f2f47eaaf4..26577548f2cd 100644 --- a/content/xul/content/src/nsXULPopupListener.cpp +++ b/content/xul/content/src/nsXULPopupListener.cpp @@ -33,7 +33,7 @@ #include "nsHTMLReflowState.h" #include "nsIObjectLoadingContent.h" #include "mozilla/Preferences.h" -#include "mozilla/dom/Element.h" +#include "mozilla/dom/FragmentOrElement.h" // for event firing in context menus #include "nsPresContext.h" @@ -52,7 +52,8 @@ using namespace mozilla; #define NS_CONTEXT_MENU_IS_MOUSEUP 1 #endif -nsXULPopupListener::nsXULPopupListener(nsIDOMElement *aElement, bool aIsContext) +nsXULPopupListener::nsXULPopupListener(mozilla::dom::Element* aElement, + bool aIsContext) : mElement(aElement), mPopupContent(nullptr), mIsContext(aIsContext) { } @@ -66,6 +67,25 @@ NS_IMPL_CYCLE_COLLECTION_2(nsXULPopupListener, mElement, mPopupContent) NS_IMPL_CYCLE_COLLECTING_ADDREF(nsXULPopupListener) NS_IMPL_CYCLE_COLLECTING_RELEASE(nsXULPopupListener) +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_BEGIN(nsXULPopupListener) + // If the owner, mElement, can be skipped, so can we. + if (tmp->mElement) { + return mozilla::dom::FragmentOrElement::CanSkip(tmp->mElement, true); + } +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_END + +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_IN_CC_BEGIN(nsXULPopupListener) + if (tmp->mElement) { + return mozilla::dom::FragmentOrElement::CanSkipInCC(tmp->mElement); + } +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_IN_CC_END + +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN(nsXULPopupListener) + if (tmp->mElement) { + return mozilla::dom::FragmentOrElement::CanSkipThis(tmp->mElement); + } +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_END + NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsXULPopupListener) NS_INTERFACE_MAP_ENTRY(nsIDOMEventListener) NS_INTERFACE_MAP_ENTRY(nsISupports) @@ -307,44 +327,37 @@ nsXULPopupListener::LaunchPopup(nsIDOMEvent* aEvent, nsIContent* aTargetContent) { nsresult rv = NS_OK; - nsAutoString type(NS_LITERAL_STRING("popup")); - if (mIsContext) - type.AssignLiteral("context"); + nsIAtom* type = mIsContext ? nsGkAtoms::context : nsGkAtoms::popup; nsAutoString identifier; - mElement->GetAttribute(type, identifier); + mElement->GetAttr(kNameSpaceID_None, type, identifier); if (identifier.IsEmpty()) { - if (type.EqualsLiteral("popup")) - mElement->GetAttribute(NS_LITERAL_STRING("menu"), identifier); - else if (type.EqualsLiteral("context")) - mElement->GetAttribute(NS_LITERAL_STRING("contextmenu"), identifier); + if (type == nsGkAtoms::popup) { + mElement->GetAttr(kNameSpaceID_None, nsGkAtoms::menu, identifier); + } else { + mElement->GetAttr(kNameSpaceID_None, nsGkAtoms::contextmenu, identifier); + } if (identifier.IsEmpty()) return rv; } // Try to find the popup content and the document. - nsCOMPtr content = do_QueryInterface(mElement); - nsCOMPtr document = content->GetDocument(); - - // Turn the document into a DOM document so we can use getElementById - nsCOMPtr domDocument = do_QueryInterface(document); - if (!domDocument) { - NS_ERROR("Popup attached to an element that isn't in XUL!"); + nsCOMPtr document = mElement->GetDocument(); + if (!document) { + NS_WARNING("No document!"); return NS_ERROR_FAILURE; } // Handle the _child case for popups and context menus - nsCOMPtr popupElement; - + nsCOMPtr popup; if (identifier.EqualsLiteral("_child")) { - nsCOMPtr popup = GetImmediateChild(content, nsGkAtoms::menupopup); - if (popup) - popupElement = do_QueryInterface(popup); - else { - nsCOMPtr nsDoc(do_QueryInterface(domDocument)); + popup = GetImmediateChild(mElement, nsGkAtoms::menupopup); + if (!popup) { + nsCOMPtr nsDoc(do_QueryInterface(document)); nsCOMPtr list; - nsDoc->GetAnonymousNodes(mElement, getter_AddRefs(list)); + nsCOMPtr el = do_QueryInterface(mElement); + nsDoc->GetAnonymousNodes(el, getter_AddRefs(list)); if (list) { uint32_t ctr,listLength; nsCOMPtr node; @@ -355,15 +368,13 @@ nsXULPopupListener::LaunchPopup(nsIDOMEvent* aEvent, nsIContent* aTargetContent) if (childContent->NodeInfo()->Equals(nsGkAtoms::menupopup, kNameSpaceID_XUL)) { - popupElement = do_QueryInterface(childContent); + popup.swap(childContent); break; } } } } - } - else if (NS_FAILED(rv = domDocument->GetElementById(identifier, - getter_AddRefs(popupElement)))) { + } else if (!(popup = document->GetElementById(identifier))) { // Use getElementById to obtain the popup content and gracefully fail if // we didn't find any popup content in the document. NS_ERROR("GetElementById had some kind of spasm."); @@ -371,12 +382,11 @@ nsXULPopupListener::LaunchPopup(nsIDOMEvent* aEvent, nsIContent* aTargetContent) } // return if no popup was found or the popup is the element itself. - if ( !popupElement || popupElement == mElement) + if (!popup || popup == mElement) return NS_OK; // Submenus can't be used as context menus or popups, bug 288763. // Similar code also in nsXULTooltipListener::GetTooltipFor. - nsCOMPtr popup = do_QueryInterface(popupElement); nsIContent* parent = popup->GetParent(); if (parent) { nsMenuFrame* menu = do_QueryFrame(parent->GetPrimaryFrame()); @@ -397,7 +407,7 @@ nsXULPopupListener::LaunchPopup(nsIDOMEvent* aEvent, nsIContent* aTargetContent) (mPopupContent->HasAttr(kNameSpaceID_None, nsGkAtoms::position) || (mPopupContent->HasAttr(kNameSpaceID_None, nsGkAtoms::popupanchor) && mPopupContent->HasAttr(kNameSpaceID_None, nsGkAtoms::popupalign)))) { - pm->ShowPopup(mPopupContent, content, EmptyString(), 0, 0, + pm->ShowPopup(mPopupContent, mElement, EmptyString(), 0, 0, false, true, false, aEvent); } else { diff --git a/content/xul/content/src/nsXULPopupListener.h b/content/xul/content/src/nsXULPopupListener.h index 311e992293ab..d401d9554918 100644 --- a/content/xul/content/src/nsXULPopupListener.h +++ b/content/xul/content/src/nsXULPopupListener.h @@ -12,7 +12,7 @@ #include "nsCOMPtr.h" -#include "nsIContent.h" +#include "mozilla/dom/Element.h" #include "nsIDOMElement.h" #include "nsIDOMMouseEvent.h" #include "nsIDOMEventListener.h" @@ -25,12 +25,12 @@ public: // false, the popup opens on left click on aElement or a descendant. If // aIsContext is true, the popup is a context menu which opens on a // context menu event. - nsXULPopupListener(nsIDOMElement *aElement, bool aIsContext); + nsXULPopupListener(mozilla::dom::Element* aElement, bool aIsContext); virtual ~nsXULPopupListener(void); // nsISupports NS_DECL_CYCLE_COLLECTING_ISUPPORTS - NS_DECL_CYCLE_COLLECTION_CLASS(nsXULPopupListener) + NS_DECL_CYCLE_COLLECTION_SKIPPABLE_CLASS(nsXULPopupListener) NS_DECL_NSIDOMEVENTLISTENER protected: @@ -48,7 +48,7 @@ private: #endif // |mElement| is the node to which this listener is attached. - nsCOMPtr mElement; + nsCOMPtr mElement; // The popup that is getting shown on top of mElement. nsCOMPtr mPopupContent;