When opening a context menu sometimes it slides to the bottom right until it hits the bottom of the screen then jumps maybe 200-400 pixels up and continues to slides. I'm not sure what the exact STR are but this is the first time I've noticed this. Nightly 18.0a1 (2012-09-02)

Possibly related to (or a dup of) bug 788528. Benoit, could this have something to do with Core "Animation"?

We need reliable STR and a regression range.

Another thing: Do you need a second monitor to reproduce this?

I'm experiencing the same thing, OSX 10.6. I have a second monitor. I noticed it around the same time as BenWa. I haven't figured out how to reproduce it yet, but it happens to me multiple times a day.

Sometimes the menu is interactive, sometimes it appears not to be.

Andrew sees this on OS X 10.6. Do others see this on other versions of OS X?

This could be the mac version of bug 624329. The comment at http://hg.mozilla.org/mozilla-central/annotate/647fabcaa951/widget/gtk2/nsWindow.cpp#l2325 explains what happens. Short version: we offset context menus by a few pixels from the mouse position and we can get into an infinite loop continually adding the offset. Also I've seen this on 10.6 with a single display.

> This could be the mac version of bug 624329. But that's an old bug, and this seems to be very new. We *really* need reliable STR and a regression range.

(Following up comment #7) Bug 788528 is reported by a couple of people on OS X 10.7.4.

(Following up comment #1) Is it possible that at least one plugin must be loaded for this bug to happen? As best I can tell we only use Core Animation for plugins.

I have this too... OSX 10.7.4 here. Happens in Aurora 17.0a2 and Nightly 2012-09-06, but I could not reproduce in Beta 16. This corresponds well to roughly when I started noticing this (when Aurora rolled over from 16->17). I found a semi-reliable way (for me at least) to trigger the action: it almost always seems to happen on the first right-click after changing tabs... subsequent right-clicks don't trigger the behavior. Right-clicks on interface elements "don't count", has to be somewhere in the page. Switch tabs, right click... switch tabs, right click... repeat. Some tab switch combos are more reliable... for example between bugzilla.mozilla.org and etherpad.mozilla.org triggers it reliably. Switching between bugzilla and github triggers it reliably. Switching between etherpad and github does *not*. o.O

I found a way to reproduce it without tab switching... simply right click on different elements (for example a link, then the background). You don't even have to left-click to make it go away, just right click back and forth. The common thread perhaps is that it triggers whenever the context menu is different in some way from the last time.

One other thing: happens in "Safe Mode" too (Help -> Restart w/ addons disabled)... suggested to me by help in #firefox.

> simply right click on different elements (for example a link, then the background) Oh, nice find! That reproduces for me, too.

> The common thread perhaps is that it triggers whenever the context > menu is different in some way from the last time. This works for me too -- though only when Firefox is on the secondary monitor. Which allowed me to find this bug's regression range: firefox-2012-08-30-03-05-31-mozilla-central firefox-2012-08-31-03-06-12-mozilla-central http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=706174d31a02&tochange=fcc533f691e9

There's nothing obvious in this range that could have triggered this bug (at least to me). Though I wonder about http://hg.mozilla.org/mozilla-central/rev/0a1f4d81635a. Someone's going to have to hg bisect. I won't have time for that until sometime next week.

Maybe http://hg.mozilla.org/mozilla-central/rev/e0ceffe737dd

(In reply to Timothy Nikkel (:tn) from comment #18) > Maybe http://hg.mozilla.org/mozilla-central/rev/e0ceffe737dd Yep, back this out fixes it for me.

Bug 786421 was uplifted to aurora, so tracking on 17 and 18.

I can't debug this at the moment but if I could I would put a break point here http://mxr.mozilla.org/mozilla-central/source/layout/xul/base/src/nsMenuPopupFrame.cpp#1202 and compare it in a buggy vs working build to see why we keep hitting that.

If we're just looking at a backout here, please attach and nominate for uplift.

I think a backout of bug 786421 would be worse than this bug.

I'll spend a few hours tomorrow looking for a plausible fix. If the patch that triggered this bug actually fixed bug 786421, we probably don't want to back it out. But it's not clear from the comments at bug 786421 that the patch *did* fix it.

Ok, so there are two patches that landed from bug 786421. The first (http://hg.mozilla.org/mozilla-central/rev/e0ceffe737dd) completely fixes that bug. It stops Resize from being called every single time we paint. The second (https://hg.mozilla.org/mozilla-central/rev/d4d50d1c9192) prevents us from doing a lot of extra work on Windows when Resize is called (but we still do an Invalidate system call at least). So we could just back out e0ceffe737dd but then on every OS we'd be making a lot of useless Resize calls every time we paint, which should be low overhead, but it's not ideal.

I think Markus' patch https://bugzilla.mozilla.org/attachment.cgi?id=581617 on bug 708278 would fix this.

I'll try it.

Yes, Markus' patch does "fix" this bug (at least in my tests). And I'd be very happy to take it (presuming that it also fixes the bug for other people). But it doesn't seem to fix the underlying problem, which to my mind is that multiple redundant calls are often made to nsMenuPopupFrame::SetPopopPosition(). I don't know how to fix this. It'd take me a long time to find out. And I'm currently extremely busy with other bugs. So what do people think? Is there consensus that it's alright to take Markus' patch, if only as a bandaid?

I think the basic sequence of events is something like this: we place the popup window at a location, the OS sends us back a move notification for this, we don't recognize the move notification as coming from our own placement of the window so we treat it like an external move notification, we add the 2 pixel context menu offset to the position like we do for all context menus and move the context menu 2 pixels down and to the left, the OS sends us a move notification for this, and the cycle repeats.

I know this doesn't fix the underlying problem, but I think it's good enough for the time being -- especially since it fixes a bad UI bug that urgently needs fixing. Markus and others, also feel free to comment. I've started tryserver builds: https://tbpl.mozilla.org/?tree=Try&pusher=smichaud@pobox.com

(In reply to Timothy Nikkel (:tn) from comment #27) > I think Markus' patch https://bugzilla.mozilla.org/attachment.cgi?id=581617 > on bug 708278 would fix this. I think so, too, but we should find out why the checks Neil mentions in bug 708278 comment 26 don't prevent this bug from happening.

(In reply to Markus Stange from comment #32) > I think so, too, but we should find out why the checks Neil mentions in bug > 708278 comment 26 don't prevent this bug from happening. We set mScreenXPos/mScreenYPos in nsMenuPopupFrame::MoveTo, then we call SetPopupPosition. SetPopupPosition doesn't change mScreenXPos/mScreenYPos, it just takes mScreenXPos/mScreenYPos and adds the context menu offset and positions the frame and the view (which positions the widget). So mScreenXPos/mScreenYPos is off by the context menu offset from the actual widget position. That's why the check in nsXULPopupManager::PopupMoved (and elsewhere) doesn't prevent this.

Oh I see. Good, that should address Neil's concerns.

Comment on attachment 660214 [details] [diff] [review] Markus' workaround You could also just subtract 2 from mScreenPosX/Y rather than converting to app units only to convert it back again.

Comment on attachment 660214 [details] [diff] [review] Markus' workaround We should also factor out the "2" and make it a named constant instead of a magic number used in two places.

(In reply to Timothy Nikkel (:tn) from comment #39) > We should also factor out the "2" and make it a named constant instead of a > magic number used in two places. I did this.(In reply to Neil Deakin from comment #38) > You could also just subtract 2 from mScreenPosX/Y rather than converting to > app units only to convert it back again. I left this as is cause it mirrored the calculation done in SetPopupPosition.

Comment on attachment 660214 [details] [diff] [review] Markus' workaround [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 786421 User impact if declined: context menus on mac sometimes slide to the bottom right of the screen Testing completed (on m-c, etc.): been on nightlies for a few days now Risk to taking this patch (and alternatives if risky): there is some risk. alternative is backing out bug 786421 and bug 357725 (which caused 786421). there is also risk to backing out something that has been in for a while now. String or UUID changes made by this patch: none

Verified on Nightly build of 20120917

Comment on attachment 660214 [details] [diff] [review] Markus' workaround We can take the small risk here on Aurora and make sure this is solid before Beta merge.

Tracy, can you please verify this against the latest Firefox 17.0a2 Aurora builds?

It's good on 17 Aurora build of 20120919