User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11 Build Identifier: In Firefox 3, nsIPopupBoxObject has a new setConsumeRollupEvent method which enables to decide whether outside clicks of popup should be consumed or not. However, the method has no effect on Mac OS X. Reproducible: Always Steps to Reproduce: 1.Open the subsequent testcase. 2.Click [ROLLUP_NO_CONSUME] button to open popup. 3.Click [TEST] button during popup display. Actual Results: Platform-dependent. - Windows: Popup closes and alert dialog opens. - Linux : Popup closes and alert dialog opens. - MacOSX : Popup closes but alert dialog does not open. Expected Results: Popup closes and alert dialog opens on all platforms. Clicks outside of a popup should NOT be eaten when we use nsIPopupBoxObject#setConsumeRollupEvent(Ci.nsIPopupBoxObject.ROLLUP_NO_CONSUME).

Is this because the popup's widget is a nsChildView and not a nsCocoaWindow? nsCocoaWindow::CaptureRollupEvents seems to do something, at least.

No, rollup consuming just isn't currently implemented on the Mac.

(In reply to comment #3) > No, rollup consuming just isn't currently implemented on the Mac. What does "rollup consuming isn't currently implemented on the Mac" mean, if not what I said?

The Mac implementation of CaptureRollupEvents ignores the consume rollups flag, and always consumes the event.

Neil or Josh: any recommendation on how to go about fixing this?

(In reply to comment #6) > Neil or Josh: any recommendation on how to go about fixing this? It's fairly easy. You just need to modify nsCocoaWindow::CaptureRollupEvents to not ignore the aConsumeRollupEvent argument. The implementations on Windows and Linux should show how this is done. They cache the value and check it when rolling up popups.

Neil, thanks for the tip on where to start! for Linux, in mozilla/widget/src/gtk2/nsWindow.cpp, in nsWindow::OnButtonPressEvent(), I see where we checked the cached value gConsumeRollupEvent after rollup: PRBool rolledUp = check_for_rollup(aEvent->window, aEvent->x_root, aEvent->y_root, PR_FALSE); if (gConsumeRollupEvent && rolledUp) return; For mac, in nsCocoaWindow.mm, is the event getting consumed in sendEvent()? It looks like the NSLeftMouseDown gets converted into a NSLeftMouseUp for the child view. If we're not consuming events, should I also generate a second event, a NSLeftMouseDown, after we call RollUpPopups()?

(In reply to comment #8) > For mac, in nsCocoaWindow.mm, is the event getting consumed in sendEvent()? It > looks like the NSLeftMouseDown gets converted into a NSLeftMouseUp for the > child view. > Mac is probably a bit trickier as nsCocoaWindow implements CaptureRollupEvents but nsChildView also does the relevant rolling up you want. See the calls to nsChildView maybeRollup. Likely the same check is needed in those places as the gtk code you show above.

Thanks for the follow up advice, Neil. That makes sense to me: we might perform the rollup in maybeRollup() by calling gRollupListener->Rollup(), but if we are not consuming the event, we could return PR_FALSE, and then in mouseDown() we would not bail out here: if ([self maybeRollup:theEvent]) return;

This works for me, but this code is tricky so I'm going to test some more. Seth - does this work for you?

Josh, thanks for whipping up a patch! I'll test it and let you know.

> I'll test it and let you know. Josh, this patch works for me. thanks again!

Great! I'll clean the patch up and get the necessary reviews.

Cleans up the implementation some.

>--- a/widget/src/cocoa/nsCocoaWindow.mm ... > PRBool gCocoaWindowMethodsSwizzled = PR_FALSE; > >+PRBool gConsumeRollupEvent; In the previous version of the patch you initialized this field. Why did you change that?

It doesn't matter what it is initialized to, I didn't want to give the impression that it mattered by picking a random value. It isn't initialized in gtk2 either.

Comment on attachment 377167 [details] [diff] [review] fix v1.1 Makes sense.

I've tested the second patch as well, and it is working for me. thanks again, josh!

pushed to mozilla-central http://hg.mozilla.org/mozilla-central/rev/7d1b9ef54d27

This method wasn't previously documented; now it is, and mentions this fix in a note: https://developer.mozilla.org/en/XUL/Method/setConsumeRollupEvent