Last Comment Bug 345896 - menus don't find menupopups in XBL
: menus don't find menupopups in XBL
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: PowerPC Mac OS X
: -- normal (vote)
: ---
Assigned To: Neil Deakin
: Neil Deakin
Depends on:
  Show dependency treegraph
Reported: 2006-07-25 12:16 PDT by Neil Deakin
Modified: 2008-07-31 03:21 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Check anonymous content also (2.03 KB, patch)
2006-07-25 12:18 PDT, Neil Deakin
no flags Details | Diff | Splinter Review
Use a utility function for this (6.50 KB, patch)
2006-08-08 15:45 PDT, Neil Deakin
no flags Details | Diff | Splinter Review
Address review comments. (6.55 KB, patch)
2006-08-14 13:26 PDT, Neil Deakin
roc: review+
roc: superreview+
Details | Diff | Splinter Review

Description Neil Deakin 2006-07-25 12:16:51 PDT
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">
Comment 1 Neil Deakin 2006-07-25 12:18:54 PDT
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.
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2006-07-25 20:41:47 PDT
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) 2006-08-06 14:57:18 PDT
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 2006-08-08 15:45:22 PDT
Created attachment 232811 [details] [diff] [review]
Use a utility function for this
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2006-08-08 20:03:34 PDT
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 2006-08-14 13:26:46 PDT
Created attachment 233627 [details] [diff] [review]
Address review comments.

Note You need to log in before you can comment on or make changes to this bug.