Bug 1702041 - Clear document.popupNode when a native menu closes. r=tnikkel

This is what non-native menus do. document.popupNode is a very awkward API; it's
global state that gets updated from multiple places and which is hard to reason
about.
For that reason, I think it's safest to stick closely to what non-native menus
are doing. I'm not aware of any user-facing breakage that this patch fixes.

Differential Revision: https://phabricator.services.mozilla.com/D110303

layout/xul/nsMenuPopupFrame.cpp

@@ -973,6 +973,25 @@ void nsMenuPopupFrame::ShowPopup(bool aIsContextMenu) {
   mShouldAutoPosition = true;
 }
 
+void nsMenuPopupFrame::ClearTriggerContentIncludingDocument() {
+  // clear the trigger content if the popup is being closed. But don't clear
+  // it if the popup is just being made invisible as a popuphiding or command
+  if (mTriggerContent) {
+    // if the popup had a trigger node set, clear the global window popup node
+    // as well
+    Document* doc = mContent->GetUncomposedDoc();
+    if (doc) {
+      if (nsPIDOMWindowOuter* win = doc->GetWindow()) {
+        nsCOMPtr<nsPIWindowRoot> root = win->GetTopWindowRoot();
+        if (root) {
+          root->SetPopupNode(nullptr);
+        }
+      }
+    }
+  }
+  mTriggerContent = nullptr;
+}
+
 void nsMenuPopupFrame::HidePopup(bool aDeselectMenu, nsPopupState aNewState) {
   NS_ASSERTION(aNewState == ePopupClosed || aNewState == ePopupInvisible,
                "popup being set to unexpected state");
@@ -984,24 +1003,11 @@ void nsMenuPopupFrame::HidePopup(bool aDeselectMenu, nsPopupState aNewState) {
       mPopupState == ePopupPositioning)
     return;
 
-  // clear the trigger content if the popup is being closed. But don't clear
-  // it if the popup is just being made invisible as a popuphiding or command
-  // event may want to retrieve it.
   if (aNewState == ePopupClosed) {
-    // if the popup had a trigger node set, clear the global window popup node
-    // as well
-    if (mTriggerContent) {
-      Document* doc = mContent->GetUncomposedDoc();
-      if (doc) {
-        if (nsPIDOMWindowOuter* win = doc->GetWindow()) {
-          nsCOMPtr<nsPIWindowRoot> root = win->GetTopWindowRoot();
-          if (root) {
-            root->SetPopupNode(nullptr);
-          }
-        }
-      }
-    }
-    mTriggerContent = nullptr;
+    // clear the trigger content if the popup is being closed. But don't clear
+    // it if the popup is just being made invisible as a popuphiding or command
+    // event may want to retrieve it.
+    ClearTriggerContentIncludingDocument();
     mAnchorContent = nullptr;
   }
 

layout/xul/nsMenuPopupFrame.h

@@ -294,6 +294,7 @@ class nsMenuPopupFrame final : public nsBoxFrame,
 
   static nsIContent* GetTriggerContent(nsMenuPopupFrame* aMenuPopupFrame);
   void ClearTriggerContent() { mTriggerContent = nullptr; }
+  void ClearTriggerContentIncludingDocument();
 
   // returns true if the popup is in a content shell, or false for a popup in
   // a chrome shell

layout/xul/nsXULPopupManager.cpp

@@ -825,9 +825,13 @@ void nsXULPopupManager::OnNativeMenuClosed() {
     return;
   }
 
-  // The native menu has closed.
-  // Null out mNativeMenu so that we don't keep it (and mContent) alive
-  // unnecessarily, and unregister ourselves first.
+  RefPtr<nsXULPopupManager> kungFuDeathGrip(this);
+
+  nsCOMPtr<nsIContent> popup = mNativeMenu->Element();
+  nsMenuPopupFrame* popupFrame = GetPopupFrameForContent(popup, true);
+  if (popupFrame) {
+    popupFrame->ClearTriggerContentIncludingDocument();
+  }
   mNativeMenu->RemoveObserver(this);
   mNativeMenu = nullptr;
 }

widget/NativeMenu.h

@@ -9,6 +9,10 @@
 #include "nsISupportsImpl.h"
 #include "Units.h"
 
+namespace mozilla::dom {
+class Element;
+}
+
 namespace mozilla::widget {
 
 class NativeMenu {
@@ -23,6 +27,9 @@ class NativeMenu {
   // Returns false if the menu wasn't open.
   virtual bool Close() = 0;
 
+  // Return this NativeMenu's DOM element.
+  virtual RefPtr<dom::Element> Element() = 0;
+
   class Observer {
    public:
     // Called when the menu opened, after popupshown.

widget/cocoa/NativeMenuMac.h

@@ -31,6 +31,7 @@ class NativeMenuMac : public NativeMenu,
   // NativeMenu
   bool ShowAsContextMenu(const mozilla::DesktopPoint& aPosition) override;
   bool Close() override;
+  RefPtr<dom::Element> Element() override;
   void AddObserver(NativeMenu::Observer* aObserver) override {
     mObservers.AppendElement(aObserver);
   }
@@ -67,7 +68,7 @@ class NativeMenuMac : public NativeMenu,
   // during ShowAsContextMenu.
   void OpenMenu(const mozilla::DesktopPoint& aPosition);
 
-  RefPtr<nsIContent> mContent;
+  RefPtr<dom::Element> mElement;
   RefPtr<nsMenuGroupOwnerX> mMenuGroupOwner;
   RefPtr<nsMenuX> mMenu;
   nsTArray<NativeMenu::Observer*> mObservers;

widget/cocoa/NativeMenuMac.mm

@@ -29,7 +29,8 @@ using dom::Element;
 
 namespace widget {
 
-NativeMenuMac::NativeMenuMac(mozilla::dom::Element* aElement) : mContainerStatusBarItem(nil) {
+NativeMenuMac::NativeMenuMac(mozilla::dom::Element* aElement)
+    : mElement(aElement), mContainerStatusBarItem(nil) {
   MOZ_RELEASE_ASSERT(aElement->IsAnyOfXULElements(nsGkAtoms::menu, nsGkAtoms::menupopup));
   mMenuGroupOwner = new nsMenuGroupOwnerX(aElement, nullptr);
   mMenu = MakeRefPtr<nsMenuX>(nullptr, mMenuGroupOwner, aElement);
@@ -270,5 +271,7 @@ void NativeMenuMac::OpenMenu(const mozilla::DesktopPoint& aPosition) {
 
 bool NativeMenuMac::Close() { return mMenu->Close(); }
 
+RefPtr<Element> NativeMenuMac::Element() { return mElement; }
+
 }  // namespace widget
 }  // namespace mozilla