93359 - bindings for modern theme create image element with children

Status: RESOLVED WORKSFORME

(SeaMonkey :: Themes, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This is a spin-off from bug 87040.

The widgetBindings.xml for the modern theme ends up creating a menu element with
child elements.  (Bug 87040 was about the fact that this caused a leak because
the frame constructor thought that nsImageBoxFrame wasn't a leaf frame, so it
created frames for the children that got dropped on the floor (with a "not a
container" assertion) in AppendFrames, which meant that their destructors were
never called, and since one of those child frames was also an nsImageBoxFrame,
it leaked an imgRequestProxy, which leaked an nsPresContext, which leaked an
nsEventStateManager, which, if you focused the document, leaked the entire
document.)

But back to the issue at hand.  To be more specific, what seems to be happening,
as I described on bug 87040, is:

The offending XUL seems to be
http://lxr.mozilla.org/seamonkey/source/themes/modern/communicator/widgetBindings.xml#53
which has CSS binding it to the menubutton-icon binding which creates anonymous
children (including a menubutton-icon-icon image, from which the leak came):
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/xul.css#576
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/bindings/menubutton.xml#53

The LXR search http://lxr.mozilla.org/seamonkey/search?string=menubutton-icon
was instructive.  I put a XUL fix, attachment 44483 [details] [diff] [review], on bug 87040.  That fix,
alone, did fix the leak (and thus presumably fixed the image element with
children -- I didn't actually check in the document inspector).  I'm not sure
it's the best way to do it, though, so I'm assiging this bug to you to figure
out what really is the right way to fix the XBL and check the correct fix in.

Comment 1

[[e-mail]]

The .menubutton-icon CSS appears to support the menulist-editable widget in
navWidgetBindings.xml which is not actually used anywhere, as there is a widget
in menulist.xml to do the job. So both these bits should be deleted.

Comment 2

[[e-mail]]

Which also explains the !important in XUL.CSS for the menulist-editable binding.
Neither navWidgetBindings or navWidgets.css appear to be used anymore.

Comment 3

[[e-mail]]

Attachment: Patch (assuming my analysis is correct :-)

Comment 4

[Joe Hewitt (gone)]

The menubutton-icon binding is certainly needed, but it just needs a name
change.  I would propose renaming the class (and all uses of it in xul) to
"menubutton-iconic"

Comment 5

[[e-mail]]

This was fixed by the fix to bug 97574.