Closed Bug 1013852 Opened 9 years ago Closed 9 years ago

Clicking doesn't focus the window sometimes

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla32
Tracking Status
firefox32 + verified

People

(Reporter: mstange, Assigned: smichaud)

References

Details

(Keywords: regression, reproducible, Whiteboard: [STR in comment #28])

Attachments

(1 file, 1 obsolete file)

Sometimes, after a different app had focus, clicking on the Firefox window doesn't give it focus. That means that clicks to the content area are ignored and the titlebar stays light gray. Clicking on the desktop and then on the Firefox window again lets me focus it. Notably, the system menu shows the Firefox menu even while the Firefox window is not focused.

I haven't been able to reproduce this consistently, but this has happened to me at least 10 times over the last two weeks. My fear is that it's been caused by bug 996848 but I don't really want to admit that.
I've seen this too.
> My fear is that it's been caused by bug 996848 but I don't really want to admit that.

I agree on both counts :-(

Let's wait for a while before we do anything (like try to backout bug 996848).  Someone may find good STR (in which case we'll know much better what we're doing as we look for a fix).  And we'll get a better idea of how often this happens.  (If it *doesn't* happen often, it'll be hard to tell whether even backing out bug 996848 has fixed it.)

For what it's worth, I haven't (yet) seen this.  But I'll keep my eyes open.
> Notably, the system menu shows the Firefox menu even while the Firefox window is not focused.

I've started seeing this occasionally when I quit Firefox.  This has been for the last few months (?), and in release versions -- so my patch for bug 996948 is clearly unrelated.  But I don't know if it's related to what you guys have been seeing.
> I've started seeing this occasionally when I quit Firefox.

More detail:  The Firefox menu continues to be displayed even after Firefox has quit.  As far as I know this glitch lasts indefinitely, but goes away as soon as you click anywhere (on the Desktop or Dock, for example).
Another thing I remember from recent FF releases (28 and 29?) that may be related (and is clearly unrelated to my patch for bug 996848):

There's some dialog where the OK button isn't highlighted until you mouse over it.  I currently can't remember which dialog, but will comment when I see it again.
It'd help to know what major version(s) of OS X you guys have seen this on.
I have 10.8.5.
I'm on 10.9.3.
Happy to run custom builds with logging or speculative guesses or anything if it would be helpful.
(In reply to Steven Michaud from comment #5)
> There's some dialog where the OK button isn't highlighted until you mouse
> over it.  I currently can't remember which dialog, but will comment when I
> see it again.

The default button not glowing issue arose after my patch for bug 839029, which we had to fix because CPU activity was really high, and we were focusing on saving battery.

We talked about this (but maybe it was only related to sheets, and dialogs are a completely different matter): I discovered that the sheet (or maybe dialog too) was receiving an activate event before it was displayed. Hence the glowing was not starting.

I think we opened a bug for the event ordering issue, which we agreed was the right way to fix this new bug, but now I can't find it.
I thought we had fixed that in bug 892547. Anyway, it's not related to this bug.

Timothy, do you have Firefox Sync enabled in your profile? I think Sync has a habit of starting nested event loops at random, so it might be related to this. I use Firefox Sync.
> I thought we had fixed that in bug 892547. Anyway, it's not related to this bug.

What I saw (and talked about in comment #5) is a different issue than bug 892547.  And like Markus said, it's probably not related.  I was just grasping for straws.

I hope the Firefox Sync angle pans out.  I don't use it.
I don't use sync, so I'm assuming it's disabled?
try build with bug 996848 backed out
https://tbpl.mozilla.org/?tree=Try&rev=f5771e6f505e
I'll try running it to see if the problem is still there.
(In reply to Timothy Nikkel (:tn) from comment #14)
> try build with bug 996848 backed out
> https://tbpl.mozilla.org/?tree=Try&rev=f5771e6f505e
> I'll try running it to see if the problem is still there.

So far the problem hasn't happened with this build, but another day or two is needed to be sure.
See bug 1016200.  Sounds related, and may be reproducible.
(In reply to Timothy Nikkel (:tn) from comment #15)
> (In reply to Timothy Nikkel (:tn) from comment #14)
> > try build with bug 996848 backed out
> > https://tbpl.mozilla.org/?tree=Try&rev=f5771e6f505e
> > I'll try running it to see if the problem is still there.
> 
> So far the problem hasn't happened with this build, but another day or two
> is needed to be sure.

Still haven't had the problem again. This all but confirms it.
Bug 1016337 _might_ also be a dupe of this, or at least related.
Status: UNCONFIRMED → NEW
Ever confirmed: true
See Also: → 1016200, 1016337
I will get to this and related bugs as soon as I can -- particularly to bug 1016200, which seems to be reproducible.  But currently I'm swamped with other things.  So it probably won't be until next week that I can really start digging in here.
Assignee: nobody → smichaud
Hardware: x86 → All
I'm definitely seeing this from time to time; FWIW, it seems to be related to page loading. Specifically, I have to wait until the page is entirely loaded before it will respond to clicks. Doesn't happen all the time, I'll try to discern STR, but you can more readily see it on heavy pages like Gmail.

I also see cases where the mouse target is above and to the left of where the mouse is actually pointing, but I'll file a separate bug for that.
Blocks: 1017296
Depends on: 1016200
Fwiw, this makes the browser completely unusable for me.  I've had to switch to a pre-bug 996848 nightly and disable updates, after fighting it for a week.  :(  If we don't think we will have this fixed soon, we should, imo, back out bug 996848.
Blocks: 996848
Severity: normal → major
Keywords: regression
I think I've been seeing this bug lately.  Here are my STR's (not 100% reproducible though)

1) Have Nightly set to open on another screen, where laptop (Mac 10.9.3 in this case) is primary machine screen.
2) Start Nightly and attempt to click on the window to drag it.

You can't. However, if you click and drag a tab just a bit, the Window is "freed up" to click on the chrome.

Also, when in the stuck state if you move Nightly via Dock icon Options to the laptop desktop, then the Nightly window then becomes clickable again.
(In reply to comment #22)

> Fwiw, this makes the browser completely unusable for me.

Boris, you seem to be worse effected by this bug than almost everyone else.  What extensions are you using?  Do you have somewhat reliable STR?  What version of OS X are you running?  On what kind of hardware?
>  What extensions are you using? 

JIT Inspector, DOM Inspector, Tab Stats.

> Do you have somewhat reliable STR? 

Start the browser.

That said, the steps in bug 1016200 comment 4 reproduce for me quite reliably (in that many of the clicks don't focus immediately) in a nightly with a clean profile.

> What version of OS X are you running?

10.9.3

> On what kind of hardware?

Recentish retina MBP.
(In reply to Steven Michaud from comment #24)
> (In reply to comment #22)
> 
> > Fwiw, this makes the browser completely unusable for me.
> 
> Boris, you seem to be worse effected by this bug than almost everyone else. 
> What extensions are you using?  Do you have somewhat reliable STR?  What
> version of OS X are you running?  On what kind of hardware?

I am similarly effected, but didn't want to pile onto the bug with a simple "yea, me too". I have switched off of Nightly and onto the release channel while waiting for this to be fixed. There aren't really STR other than to use the browser, there are constantly focus issues. I am also running 10.9.3 and I have a mid-2012 retina MBP
Looks like I'll be able to start serious work on this bug sooner than I expected -- this afternoon instead of next week.  I'll give myself to the end of next week to find a fix.  If I can't, I'll back out my 996848, and hopefully fall back to a combination of my bandaid patch for bug 959281 and smaug's patch for bug 953435.  (Otherwise we'd reopen a bunch of *other* nasty bugs.)
I've found good (and very simple) STR for this bug:

1) Paste following into the Web Console and hit enter:

   setInterval(function () { for (var i=0; i<1000; i++) console.log("foobar"); }, 0);

   Or if you've already done this, make sure the tab is displayed that contains the Web Console.

2) Click on the Desktop to make Firefox lose the app focus.

3) Click on the "+" symbol at the right end of the tab bar to create a new tab.

   Especially if there are already lots of tabs, Firefox often gets stuck only part of the way
   to getting the app focus.  Sometimes the window just has an inactive appearance.  Sometimes
   the Firefox menu also doesn't get displayed.

I tested on OS X 10.8.5.  I imagine this would "work" on any version of OS X.
Keywords: reproducible
Whiteboard: [STR in comment #28]
My STR doesn't "work" with tn's tryserver build from comment #14, which has my patch for bug 996848 backed out.  That pins the blame for this bug pretty unambiguously :-(
Attached patch Possible fix (obsolete) — Splinter Review
I came up with a fix for this more quickly that I thought.

I've started a set of tryserver builds:
https://tbpl.mozilla.org/?tree=Try&rev=37de3af8eb90

Once the 64-bit opt build becomes available, please try it out.
Seems to work fine for me with the STR I used to determine the regression range. So far Nightly received focus every time I tried.
This patch just changes a comment.

On further research I found that this bug *doesn't* result from NSAppKitDefined and/or NSSystemDefined events being processed in the wrong order.  Rather, sending such an event to the event dispatcher target prevents it from being processed at all as a Cocoa event (from being processed by -[NSEvent sendEvent:]).
Attachment #8432731 - Flags: review?(mstange)
Attachment #8431946 - Attachment is obsolete: true
Comment on attachment 8432731 [details]
Fix rev1 (change comment)

I don't really understand how processing too many Cocoa events here will break things, but if this works, great.
Attachment #8432731 - Flags: review?(mstange) → review+
There are two main considerations when processing events in nsAppShell::ProcessNextNativeEvent() while aMayWait is false.

1) We don't want to spin the native event loop (for what I hope are obvious reasons).
2) We don't want to spend "too much time" on anything, to avoid nsIRunnable event starvation in nsThread::ProcessNextEvent() (which indirectly calls nsAppShell::ProcessNextNativeEvent().

This bug adds a third:

Some kinds of events (probably including all user events, like keyboard and mouse events) won't get handled correctly if they're just sent to the event dispatcher target, rather than (indirectly) processed through -[NSApplication sendEvent:] (called from -[NSApplication run] in nsAppShell::Run()).
Oh, SendEventToEventTarget doesn't go through -[NSApplication sendEvent:]? Now this makes more sense to me.
> Oh, SendEventToEventTarget doesn't go through -[NSApplication sendEvent:]?

No, it doesn't call -[NSApplication sendEvent:], even indirectly.  But DPSNextEvent() (called from -[NSApplication nextEventMatchingMask:...]) and -[NSApplication sendEvent:] can call SendEventToEventTarget().  So at first I thought it was fine to send all non-user events straight to SendEventToEventTarget(), which seems somehow to be more "fundamental".

But that's not always so.  This bug shows it for NSAppKitDefined and NSSystemDefined events.

And there may be others, which we'll have to find by trial and error.

I do hope, though, that with user events and NSAppKitDefined and NSSystemDefined events, we've covered most of the ground.
So, what are the kinds of events we do want to process at that point, and why? I think the code could use a comment about why we don't just bail out for all events there.
(In reply to comment #38)

I don't have a full answer yet.  I've already included what I know in comments.

It *might* be possible for us to just do nothing (to not process any events) in nsAppShell::ProcessNextNativeEvent() when aMayWait is false.  I noticed, for example, that doing this made this bug go away.

But somehow that seems too radical, at least to me, at least for now.
No longer depends on: 1016200
https://hg.mozilla.org/mozilla-central/rev/5d31a639e5a8
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
See Also: → 1011706
Verified as fixed using the following environment:

FF32.0b8
Build Id:20140818191513
OS:Mac Os X 1.8.5
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.