392652 - nsXULPopupManager::Rollup() should synchronously hide popups

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Josh Aas]

Right now nsXULPopupManager::Rollup() hides popups asynchronously. This can be problematic in at least the following order of events...

1. right mouse down event
2. context menu event
3. right mouse down event
4. context menu event

The problem here is that after event #2 a popup is showing, then event #3 results in an async popup hide, which is processed *after* event #4. You end up with no context menu open when there should be one open from #4.

nsXULPopupManager::Rollup() should probably do synchronous hiding. Apparently we can do that so long as nobody calls it from certain bad places. I don't remember what all of those places are so I'll let somebody who knows list that out. Enn?

Comment 1

[Neil Deakin]

Attachment: call rollup synchronously

I looked at all the callers of Rollup. There are only a couple outside of the widget code which I've changed in this patch. The others are in widget which are ok with calling this synchronously.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 279781 [details] [diff] [review]
call rollup synchronously

Looks reasonable

Comment 3

[Mike Connor [:mconnor]]

Comment on attachment 279781 [details] [diff] [review]
call rollup synchronously

a=mconnor on behalf of drivers

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Oops, this was landed long ago:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-09-18+08%3A00%3A00&maxdate=2007-09-18+08%3A00%3A00&cvsroot=%2Fcvsroot