menus don't find menupopups in XBL

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
13 years ago
11 years ago

People

(Reporter: enndeakin, Assigned: enndeakin)

Tracking

Trunk
PowerPC
macOS
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

13 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

13 years ago
Posted 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).
(Assignee)

Updated

13 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

13 years ago
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

13 years ago
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

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

Updated

11 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.