Bug 801609 - Simplify popup and contextmenu handling and make the listener skippable, r=mccr8
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Tue, 16 Oct 2012 15:37:26 +0300
changeset 110512 fc883f5a1a0830fa405879998e040cd8cb651c0b
parent 110511 8f145599e4bf81de2d0421c071b5863175a585dc
child 110513 b3d5c1bcf8470f13703aca76c7890ac7abd8df4e
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersmccr8
bugs801609
milestone19.0a1
Bug 801609 - Simplify popup and contextmenu handling and make the listener skippable, r=mccr8
content/base/src/FragmentOrElement.cpp
content/base/src/nsGkAtomList.h
content/canvas/test/test_mozGetAsFile.html
content/xul/content/src/nsXULElement.cpp
content/xul/content/src/nsXULElement.h
content/xul/content/src/nsXULPopupListener.cpp
content/xul/content/src/nsXULPopupListener.h
dom/base/test/test_gsp-standards.html
--- a/content/base/src/FragmentOrElement.cpp
+++ b/content/base/src/FragmentOrElement.cpp
@@ -1160,19 +1160,16 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Fr
   nsINode::Unlink(tmp);
 
   if (tmp->HasProperties()) {
     if (tmp->IsHTML()) {
       tmp->DeleteProperty(nsGkAtoms::microdataProperties);
       tmp->DeleteProperty(nsGkAtoms::itemtype);
       tmp->DeleteProperty(nsGkAtoms::itemref);
       tmp->DeleteProperty(nsGkAtoms::itemprop);
-    } else if (tmp->IsXUL()) {
-      tmp->DeleteProperty(nsGkAtoms::contextmenulistener);
-      tmp->DeleteProperty(nsGkAtoms::popuplistener);
     }
   }
 
   // Unlink child content (and unbind our subtree).
   if (tmp->UnoptimizableCCNode() || !nsCCUncollectableMarker::sGeneration) {
     uint32_t childCount = tmp->mAttrsAndChildren.ChildCount();
     if (childCount) {
       // Don't allow script to run while we're unbinding everything.
@@ -1703,23 +1700,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
                                          (tmp->GetProperty(nsGkAtoms::microdataProperties));
       cb.NoteXPCOMChild(property);
       property = static_cast<nsISupports*>(tmp->GetProperty(nsGkAtoms::itemref));
       cb.NoteXPCOMChild(property);
       property = static_cast<nsISupports*>(tmp->GetProperty(nsGkAtoms::itemprop));
       cb.NoteXPCOMChild(property);
       property = static_cast<nsISupports*>(tmp->GetProperty(nsGkAtoms::itemtype));
       cb.NoteXPCOMChild(property);
-    } else if (tmp->IsXUL()) {
-      nsISupports* property = static_cast<nsISupports*>
-                                         (tmp->GetProperty(nsGkAtoms::contextmenulistener));
-      cb.NoteXPCOMChild(property);
-      property = static_cast<nsISupports*>
-                            (tmp->GetProperty(nsGkAtoms::popuplistener));
-      cb.NoteXPCOMChild(property);
     }
   }
 
   // Traverse attribute names and child content.
   {
     uint32_t i;
     uint32_t attrs = tmp->mAttrsAndChildren.AttrCount();
     for (i = 0; i < attrs; i++) {
--- a/content/base/src/nsGkAtomList.h
+++ b/content/base/src/nsGkAtomList.h
@@ -221,17 +221,16 @@ GK_ATOM(contenteditable, "contenteditabl
 GK_ATOM(headerContentDisposition, "content-disposition")
 GK_ATOM(headerContentLanguage, "content-language")
 GK_ATOM(contentLocation, "content-location")
 GK_ATOM(headerContentScriptType, "content-script-type")
 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")
 GK_ATOM(copy, "copy")
 GK_ATOM(copyOf, "copy-of")
 GK_ATOM(count, "count")
 GK_ATOM(crop, "crop")
 GK_ATOM(crossorigin, "crossorigin")
@@ -828,17 +827,16 @@ GK_ATOM(pointSize, "point-size")
 GK_ATOM(poly, "poly")
 GK_ATOM(polygon, "polygon")
 GK_ATOM(popup, "popup")
 GK_ATOM(popupalign, "popupalign")
 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")
 GK_ATOM(popupsinherittooltip, "popupsinherittooltip")
 GK_ATOM(position, "position")
 GK_ATOM(poster, "poster")
 GK_ATOM(pre, "pre")
 GK_ATOM(preceding, "preceding")
--- a/content/xul/content/src/nsXULElement.cpp
+++ b/content/xul/content/src/nsXULElement.cpp
@@ -1549,68 +1549,36 @@ nsXULElement::GetBindingParent() const
 }
 
 bool
 nsXULElement::IsNodeOfType(uint32_t aFlags) const
 {
     return !(aFlags & ~eCONTENT);
 }
 
-static void
-PopupListenerPropertyDtor(void* aObject, nsIAtom* aPropertyName,
-                          void* aPropertyValue, void* aData)
-{
-  nsIDOMEventListener* listener =
-    static_cast<nsIDOMEventListener*>(aPropertyValue);
-  if (!listener) {
-    return;
-  }
-  nsEventListenerManager* manager = static_cast<nsINode*>(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<nsIDOMEventListener> popupListener =
-        static_cast<nsIDOMEventListener*>(GetProperty(listenerAtom));
-    if (popupListener) {
-        // Popup listener is already installed.
+    if (HasFlag(listenerFlag)) {
         return NS_OK;
     }
 
-    popupListener = new nsXULPopupListener(this, isContext);
+    nsCOMPtr<nsIDOMEventListener> 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,
                                       NS_LITERAL_STRING("contextmenu"),
                                       NS_EVENT_FLAG_BUBBLE |
                                       NS_EVENT_FLAG_SYSTEM_EVENT);
     } else {
       manager->AddEventListenerByType(listener,
--- a/content/xul/content/src/nsXULElement.h
+++ b/content/xul/content/src/nsXULElement.h
@@ -316,21 +316,23 @@ public:
   The XUL element.
 
  */
 
 #define XUL_ELEMENT_FLAG_BIT(n_) NODE_FLAG_BIT(ELEMENT_TYPE_SPECIFIC_BITS_OFFSET + (n_))
 
 // 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
 
 class nsScriptEventHandlerOwnerTearoff;
 
 class nsXULElement : public nsStyledElement, public nsIDOMXULElement
 {
 public:
--- a/content/xul/content/src/nsXULPopupListener.cpp
+++ b/content/xul/content/src/nsXULPopupListener.cpp
@@ -28,17 +28,17 @@
 #include "nsServiceManagerUtils.h"
 #include "nsIPrincipal.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsLayoutUtils.h"
 #include "nsFrameManager.h"
 #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"
 #include "nsIPresShell.h"
 #include "nsFocusManager.h"
 #include "nsPIDOMWindow.h"
 #include "nsIViewManager.h"
 #include "nsError.h"
@@ -47,30 +47,50 @@
 using namespace mozilla;
 
 // on win32 and os/2, context menus come up on mouse up. On other platforms,
 // they appear on mouse down. Certain bits of code care about this difference.
 #if defined(XP_WIN) || defined(XP_OS2)
 #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)
 {
 }
 
 nsXULPopupListener::~nsXULPopupListener(void)
 {
   ClosePopup();
 }
 
 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)
 NS_INTERFACE_MAP_END
 
 ////////////////////////////////////////////////////////////////
 // nsIDOMEventListener
 
@@ -302,86 +322,76 @@ GetImmediateChild(nsIContent* aContent, 
 // (popup, context) and uses that attribute's value as an ID for
 // the popup content in the document.
 //
 nsresult
 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<nsIContent> content = do_QueryInterface(mElement);
-  nsCOMPtr<nsIDocument> document = content->GetDocument();
-
-  // Turn the document into a DOM document so we can use getElementById
-  nsCOMPtr<nsIDOMDocument> domDocument = do_QueryInterface(document);
-  if (!domDocument) {
-    NS_ERROR("Popup attached to an element that isn't in XUL!");
+  nsCOMPtr<nsIDocument> document = mElement->GetDocument();
+  if (!document) {
+    NS_WARNING("No document!");
     return NS_ERROR_FAILURE;
   }
 
   // Handle the _child case for popups and context menus
-  nsCOMPtr<nsIDOMElement> popupElement;
-
+  nsCOMPtr<nsIContent> popup;
   if (identifier.EqualsLiteral("_child")) {
-    nsCOMPtr<nsIContent> popup = GetImmediateChild(content, nsGkAtoms::menupopup);
-    if (popup)
-      popupElement = do_QueryInterface(popup);
-    else {
-      nsCOMPtr<nsIDOMDocumentXBL> nsDoc(do_QueryInterface(domDocument));
+    popup = GetImmediateChild(mElement, nsGkAtoms::menupopup);
+    if (!popup) {
+      nsCOMPtr<nsIDOMDocumentXBL> nsDoc(do_QueryInterface(document));
       nsCOMPtr<nsIDOMNodeList> list;
-      nsDoc->GetAnonymousNodes(mElement, getter_AddRefs(list));
+      nsCOMPtr<nsIDOMElement> el = do_QueryInterface(mElement);
+      nsDoc->GetAnonymousNodes(el, getter_AddRefs(list));
       if (list) {
         uint32_t ctr,listLength;
         nsCOMPtr<nsIDOMNode> node;
         list->GetLength(&listLength);
         for (ctr = 0; ctr < listLength; ctr++) {
           list->Item(ctr, getter_AddRefs(node));
           nsCOMPtr<nsIContent> childContent(do_QueryInterface(node));
 
           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.");
     return rv;
   }
 
   // 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<nsIContent> popup = do_QueryInterface(popupElement);
   nsIContent* parent = popup->GetParent();
   if (parent) {
     nsMenuFrame* menu = do_QueryFrame(parent->GetPrimaryFrame());
     if (menu)
       return NS_OK;
   }
 
   nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
@@ -392,17 +402,17 @@ nsXULPopupListener::LaunchPopup(nsIDOMEv
   // popupanchor and popupalign attributes are used, anchor the popup to the
   // element, otherwise just open it at the screen position where the mouse
   // was clicked. Context menus always open at the mouse position.
   mPopupContent = popup;
   if (!mIsContext &&
       (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 {
     int32_t xPos = 0, yPos = 0;
     nsCOMPtr<nsIDOMMouseEvent> mouseEvent = do_QueryInterface(aEvent);
     mouseEvent->GetScreenX(&xPos);
     mouseEvent->GetScreenY(&yPos);
 
--- a/content/xul/content/src/nsXULPopupListener.h
+++ b/content/xul/content/src/nsXULPopupListener.h
@@ -7,35 +7,35 @@
  * This is the popup listener implementation for popup menus and context menus.
  */
 
 #ifndef nsXULPopupListener_h___
 #define nsXULPopupListener_h___
 
 #include "nsCOMPtr.h"
 
-#include "nsIContent.h"
+#include "mozilla/dom/Element.h"
 #include "nsIDOMElement.h"
 #include "nsIDOMMouseEvent.h"
 #include "nsIDOMEventListener.h"
 #include "nsCycleCollectionParticipant.h"
 
 class nsXULPopupListener : public nsIDOMEventListener
 {
 public:
     // aElement is the element that the popup is attached to. If aIsContext is
     // 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:
     // open the popup. aEvent is the event that triggered the popup such as
     // a mouse click and aTargetContent is the target of this event.
     virtual nsresult LaunchPopup(nsIDOMEvent* aEvent, nsIContent* aTargetContent);
 
     // close the popup when the listener goes away
@@ -43,17 +43,17 @@ protected:
 
 private:
 #ifndef NS_CONTEXT_MENU_IS_MOUSEUP
     // When a context menu is opened, focus the target of the contextmenu event.
     nsresult FireFocusOnTargetContent(nsIDOMNode* aTargetNode);
 #endif
 
     // |mElement| is the node to which this listener is attached.
-    nsCOMPtr<nsIDOMElement> mElement;
+    nsCOMPtr<mozilla::dom::Element> mElement;
 
     // The popup that is getting shown on top of mElement.
     nsCOMPtr<nsIContent> mPopupContent; 
 
     // true if a context popup
     bool mIsContext;
 };