The default bug view has changed. See this FAQ.

menus don't find menupopups in XBL

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Neil Deakin, Assigned: Neil Deakin)

Tracking

Trunk
PowerPC
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

11 years ago
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>
(Assignee)

Comment 1

11 years ago
Created attachment 230616 [details] [diff] [review]
Check anonymous content also

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).
(Assignee)

Updated

11 years ago
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?
(Assignee)

Comment 4

11 years ago
Created attachment 232811 [details] [diff] [review]
Use a utility function for this
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.
(Assignee)

Comment 6

11 years ago
Created attachment 233627 [details] [diff] [review]
Address review comments.
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+
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

9 years ago
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.