830623 - Social API moves focus into the url bar whenever I get a new message

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Rob Arnold [:robarnold]]

Haven't seen this on Windows but it happens all the time to me on OSX. I disabled the integration on one of my machines and I haven't seen it on that machine since. It doesn't seem to always occur but I believe it happens when you get an IM on a window that is minimized (I could not repro on a maximized window). My sidebar is off, but the chat and notification jewels are enabled.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Shane, nothing here leaps out at me, and you both have a mac and recently looked at focus issues, so adding you to CC.  Rob has indicated he has disabled the entire feature due to how disruptive this is, and assuming other users feel the same, we should probably treat this as a priority.

Comment 2

[Shane Caraveo (:mixedpuppy)]

My build isn't working right now to do some testing, but I am guessing this has something to do with the focus advancement happening in socialchat.xml.  Perhaps when the chat is minimized, there is nothing to advance into, an error occurs, and focus gets reset.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Attachment: Don't attempt to focus a minimized chat.

A similar problem happens on Linux - focus doesn't go to the URL bar, but seems to go to the tab itself.

The problem, as I understand it, is that the focusManager can't find any focusable elements in the minimized chatbox, so selects another element to focus instead.  A simple fix for this specific issue is to have socialchat.xml explicitly check for the selectedChat being minimized, and not attempting to set focus when that is the case.  (FWIW, I explicitly tried focusing the iframe and iframe.contentDocument.documentElement, but the same problem exists.)

This patch does exactly that.  It still leaves us with the problem that if the chat is not minimized, focus gets "stolen" to the chat window, but that already has a bug and is quite a different problem.

Gavin - any thoughts?

Comment 4

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 714177 [details] [diff] [review]
Don't attempt to focus a minimized chat.

This looks good to me.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Hmm, advanceFocusIntoSubtree is really mostly just a backwards compat/convenience wrapper around nsIFocusManager::MoveFocus. MoveFocus allows you to pass flags controlling the behavior, like FLAG_NOPARENTFRAME. I wonder whether we should just do something like that instead, to also handle the case where the chat is unminimized but doesn't contain and focusable children.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 714177 [details] [diff] [review]
Don't attempt to focus a minimized chat.

This is a reasonable work-around for the immediate issue, though.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

The patch in bug 831489 covers this issue too, right?

Comment 8

[Mark Hammond [:markh] [:mhammond]]

I just verified things work as expected on Linux with the patch to bug 831489