396186 - reverse async context menu events after bug 392652 is fixed

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Josh Aas]

Attachment: fix v1.0

After bug 392652 is fixed, we won't need to send async context menu events from child widgets any more (bug 389542). Sending the cm events async was a necessary hack to work around popups closing asynchronously.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

Josh, in order to review this patch I need to test to see if it makes
bug 381477 and/or bug 385408 start happening again.  So I'd like to
put this off for a bit (until next Monday).

If this isn't OK, let me know.

Comment 2

[Josh Aas]

Yeah, go ahead and wait.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 280901 [details] [diff] [review]
fix v1.0

Bug 385408 remains fixed (on OS X) with this patch, but bug 381477
starts happening again.

I will try fiddling with the code that sends an NS_CONTEXTMENU event
to Gecko.  But I suspect we'll need to keep doing this asynchronously
in Minefield/Firefox.

I'm going to assign this bug to myself.  Please let me know if you
have any objections, Josh.

Comment 4

[Neil Deakin]

Are there still plans to fix this?

Comment 5

[Steven Michaud [:smichaud] (Retired)]

The last time I checked (see comment #3), applying this bug's patch
caused bug 381477 to start happening again.  But now I've checked
again (altering code pulled at the beginning of this week), and this
seems to no longer be true.  Likewise with bug 385408 -- this bug's
patch still doesn't cause it to start happening again.

So I suppose it's now safe to land it.  But if there's going to be
another beta after beta3 (and I sincerely hope so), I'd prefer to
postpone landing this patch until after beta3.  In any case I don't
believe this change is urgent -- I don't know of any user-visible
problems that this bug's patch will resolve.  So it may be best to
save it until after the Firefox 3.0 release.

(Interestingly, the problem described in bug 392389 comment #11 also
no longer happens.  I'd guess a single patch is responsible for both
changes.  I'll try to find out which it is.)

Comment 6

[Steven Michaud [:smichaud] (Retired)]

> (Interestingly, the problem described in bug 392389 comment #11 also
> no longer happens.  I'd guess a single patch is responsible for both
> changes.  I'll try to find out which it is.)

The problem I described in bug 392389 comment #11 stopped happening as
of the 2007-12-18-04-trunk nightly.  I suppose the patch for bug
407876 must have fixed it.

I haven't yet checked if the patch for bug 407876 also caused applying
this bug's patch to no longer re-open bug 381477.

Comment 7

[Josh Aas]

Attachment: fix v1.0.1

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 297284 [details] [diff] [review]
fix v1.0.1

After my tests on 1-11 (comment #5), I no longer have any specific
objections to this change.  And if we're going to include it in
Firefox 3, the sooner we land it the better ... to see what fallout we
may get, and (perhaps) to uncover other bugs.

Comment 9

[Josh Aas]

landed on trunk