345896 - menus don't find menupopups in XBL

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Neil Deakin]

I want to be able to provide a <menupopup> defined in XBL for a menu, however the menu frame code only looks for non-anonymous children. Example:

<binding id="blah" display="xul:menu">
  <content>
    <xul:menupopup>
      ...
    </xul:menupopup>
  </content>
</binding>

Comment 1

[Neil Deakin]

Attachment: Check anonymous content also

This patch looks for non-anonymous menupopup elements and if not found, looks for anonymous menupopups next.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I don't really know this code...  If you can find anyone who does (Neil Rashbrook and roc may be the best initial candidates), that would be great.  Otherwise I'll try to understand it well enough to review, but that would be really slow (weeks at best).

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

How about abstracting this out into a new method
nsIContent* nsContentUtils::FindFirstChildWithResolvedTag(nsIContent*, nsIAtom*)
with an XXX comment that really, we should be returning the first child, but we can't currently do that because XBL currently doesn't tell us the relative ordering of anonymous vs explicit children, and this should be fixed.

Also, don't we need to check the namespace somehow?

Comment 4

[Neil Deakin]

Attachment: Use a utility function for this

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 232811 [details] [diff] [review]
Use a utility function for this

+    nsIContent* childContent;
+    nsCOMPtr<nsIDOMNode> childNode;
+    children->Item(i, getter_AddRefs(childNode));
+    CallQueryInterface(childNode, &childContent);
+    xblService->ResolveTag(childContent, &namespaceID, getter_AddRefs(tag));
+    if (tag == aTag && namespaceID == aNamespace) {
+      return childContent;
+    }

This seems to be leaking childContent. Use an nsCOMPtr and do_QueryInterface instead.

Mention in the comment that the returned pointer has not been addreffed.

Comment 6

[Neil Deakin]

Attachment: Address review comments.