337379 - Dynamic submenus under toolbarbutton crash browser [@ nsMenuFrame::Notify] or cause a hang on shutdown

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Eric Jung [:ericjung]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: 

I'm trying to dynmically build a <menu/> under a <menupopup/>.

After the following code is executed in the browser (via overlay or
Ted's Real-Time XUL Editor) and the submenu is popped-up:

  * No submenu appears
  * Submenus throughout the browser no longer function (e.g.,
View->Toolbars in FF or View->Show/Hide in Mozilla Suite)
  * The browser process does not terminate when the last window is
closed--the process must be manually killed
  * FF sometimes crashes

Tested in Mozilla 1.7.12 and FF 1.5.0.3 on Win XP:

<?xml version="1.0"?>
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
<window id="yourwindow"
xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
<script>
<![CDATA[
function build(parent) {
  dump("in build()\n");
  while (parent.hasChildNodes())
    parent.removeChild(parent.firstChild);

  var a = document.createElement("menuitem");
  a.setAttribute("label", "a");
  parent.appendChild(a);

  var submenu = document.createElement("menu");
  submenu.setAttribute("label", "submenu");
  
  var submenupopup = document.createElement("menupopup");
  //submenupopup.setAttribute("onpopupshowing", function(){return false;});
  submenu.appendChild(submenupopup);
      
  var b = document.createElement("menuitem");
  b.setAttribute("label", "b");
  submenupopup.appendChild(b);
 
  parent.appendChild(submenu); 
}
]]>
</script>
<toolbarbutton label="click me" type="menu">
  <menupopup onpopupshowing="build(this);"/>
</toolbarbutton>
</window>

You'll notice (in FF, at least) that the dump statement is executed
once for the parent <menupopup/> and once for the sub- <menupopup/>
even if I set onpopupshowing for the submenupopup to an empty fcn.

See more commentary here:
  * http://mozdev.org/pipermail/project_owners/2006-May/007943.html
  * http://forums.mozillazine.org/viewtopic.php?t=295084
  * bug 322023


Reproducible: Always

Steps to Reproduce:
Execute code specified in "Details" section and be sure to hover over the submenu so the popupshowing event is fired at least once.
Actual Results:  
* No submenu appears
* Submenus throughout the browser no longer function (e.g.,
View->Toolbars in FF or View->Show/Hide in Mozilla Suite)
* The browser process does not terminate when the last window is
closed--the process must be manually killed
* FF sometimes crashes

Expected Results:  
* Submenu with single menuitem "b" should appear

Per Nickolay:

> The crash happens dereferencing null mOpenTimer at
> nsCOMPtr<nsITimer>::operator->()  Line 849
> nsMenuFrame::Notify(nsITimer * aTimer=0x052b08f0)  Line 1327
> nsMenuTimerMediator::Notify(nsITimer * aTimer=0x052b08f0)  Line 2097
> nsTimerImpl::Fire()  Line 405
> handleTimerEvent(PLEvent * aEvent=0x045d4bd0)  Line 470
> PL_HandleEvent(PLEvent * self=0x045d4bd0)  Line 688

Comment 1

[Nickolay_Ponomarev]

Adam: at the bottom of this bug report is the relevant part of the stack from my less than a week old debug build. Removing stackwanted keyword.

Confirming the bug, both the crash and hang on shutdown happened to me.

Comment 2

[Andrew Schultz]

This also apparently happens in the SeaMonkey UI if it can't get a string for one of the submenus (currently happens for the es-ES localization).

the line it crashes on is

1327      mOpenTimer->Cancel();

mOpenTimer is null, but it was used earlier in the method
   if (aTimer == mOpenTimer.get()) {

So mOpenTimer is getting freed while the method is executing

Comment 3

[Marc Bejarano]

is the problem still there if you back out the patch from bug 241733?

Comment 4

[Mike Kaply [:mkaply]]

I see this as well and the 241733 patch isn't easily backed out anymore (on FF 2.0.0.4)

Comment 5

[Mike Kaply [:mkaply]]

I backed out 241733 and it did NOT fix the problem. So that does not appear to be the source of the problem.

Comment 6

[Nickolay_Ponomarev]

I don't see crashes or hangs with the patch for bug 279703 landed. Eric, if you can't reproduce anymore, this bug probably should be closed, although it would be nice to get an automated test checked in.

The submenu still doesn't open for me, but it's a separate bug, which should be filed, unless it already has been filed.

Comment 7

[Nickolay_Ponomarev]

Marking as fixed and setting in-testsuite?, since there were no objections.

Comment 8

[Eric Jung [:ericjung]]

(In reply to comment #6)
> I don't see crashes or hangs with the patch for bug 279703 landed. Eric, if you
> can't reproduce anymore, this bug probably should be closed

Sorry, I won't be able to test this.