Closed Bug 345896 Opened 14 years ago Closed 14 years ago

menus don't find menupopups in XBL

Categories

(Core :: XUL, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: enndeakin, Assigned: enndeakin)

Details

Attachments

(1 file, 2 obsolete files)

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>
Attached patch Check anonymous content also (obsolete) — Splinter Review
This patch looks for non-anonymous menupopup elements and if not found, looks for anonymous menupopups next.
Attachment #230616 - Flags: superreview?(bzbarsky)
Attachment #230616 - Flags: review?(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).
Attachment #230616 - Flags: superreview?(roc)
Attachment #230616 - Flags: superreview?(bzbarsky)
Attachment #230616 - Flags: review?(roc)
Attachment #230616 - Flags: review?(bzbarsky)
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?
Attached patch Use a utility function for this (obsolete) — Splinter Review
Attachment #230616 - Attachment is obsolete: true
Attachment #232811 - Flags: superreview?(roc)
Attachment #232811 - Flags: review?(roc)
Attachment #230616 - Flags: superreview?(roc)
Attachment #230616 - Flags: review?(roc)
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.
Attachment #232811 - Attachment is obsolete: true
Attachment #233627 - Flags: superreview?(roc)
Attachment #233627 - Flags: review?(roc)
Attachment #232811 - Flags: superreview?(roc)
Attachment #232811 - Flags: review?(roc)
Attachment #233627 - Flags: superreview?(roc)
Attachment #233627 - Flags: superreview+
Attachment #233627 - Flags: review?(roc)
Attachment #233627 - Flags: review+
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.menus → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.