Closed Bug 563321 Opened 11 years ago Closed 11 years ago

focus lost when Camino opens a new foreground tab

Categories

(Camino Graveyard :: General, defect)

1.9.2 Branch
x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Camino2.1

People

(Reporter: phiw2, Assigned: stuart.morgan+bugzilla)

References

Details

(Keywords: regression)

Attachments

(1 file)

When a new foreground tab is opened its content part has no focus at all. Paging down (spacebar or arrow/page down keys) as no effect, tabbing to links/form controls leads to nowhere. Cycling between open windows brings back the focus in the offending tab. Similarly, a cookie sheet that pops up (or a js alert) restores the focus.

STR:

1. Click a link in any app (opens in new, foreground tab) - mail.app, colloquy, …
or
1a.  double-clicking a .webloc file in the Finder

AR: a new, foreground tab is opened, but nothing is focused. Keyboard interaction with the content fails completely (but keyboard shortcuts still work).
ER: a new, foreground tab opens, the user can begin interacting with the page from the keyboard.

This is (probably) due to the changes in focus handling from bug 178324.

enn, smaug can you help us figuring out if Core/embedding is broken or if we need to make additional changes on our end ?
similarly:
*  Cmd-shift click a link
*  Cmd-shift click a bookmark on the BM bar

opens a new foreground tab with no focus
I have no idea how Camino embeds Gecko, but I'd guess that you need to
activate the opened tab/window using nsIEmbeddingSiteWindow::setFocus() or
nsIWindowWatcher::activeWindow or nsIFocusManager API. 

But since the behavior has changed, might be better to fix the regression at
least on trunk.

Enn would know better.
Enn, I think the embedding focus handling regression has been fixed on GTK2 widgets,  but are you, or someone else working on to bring back the old
behavior also on other widgets?
Flags: camino2.1a1?
Target Milestone: --- → Camino2.1
So we do activate the new tab's contents when it is created, just as we always have, via nsIWebBrowserFocus::Activate.

The problem is that when we call it the first time, when we get into nsFocusManager::Focus (via nsFocusManager::WindowRaised), it bails in the first set of checks because presShell is null. Then every subsequent call to nsFocusManager::WindowRaised bails in the if (mActiveWindow == window) check says it's already been focused, even though the actual focusing never happened. Charmingly, the comment for that check says it's "a special case for Windows", and yet is being applied to Camino.

I'll see if I can find a way to rework our focus to work around this since there's been no response here in over a month (and it sounds from comment 2 like this wouldn't get fixed for 1.9.2 anyway). I don't know yet if this is actually broken behavior in Core for embedding, or if the requirements for handling focus changed and nobody told us.
Thanks for debugging. Neil, could you please look at this?
Attached patch workaroundSplinter Review
Assignee: nobody → stuart.morgan+bugzilla
Status: NEW → ASSIGNED
Attachment #452556 - Flags: superreview?(mikepinkerton)
So what things are available at the point that you call Activate?

The DOMWindow, document and content nodes, but no presshell yet?

Was there no presshell in earlier versions (pre-focus-manager) when you called Activate as well? Presumably, no frames were available either. In what way was the focus updated in gecko when the presshell/frames were eventually available?
Comment on attachment 452556 [details] [diff] [review]
workaround

sr=pink

but I'd rather we fix this correctly at the gecko level than hack around it. This seems quite fragile. I don't suppose we can get the bug that regressed this backed out until it's fixed, eh?
Attachment #452556 - Flags: superreview?(mikepinkerton) → superreview+
What regressed this was a large-scale focus rewrite, that, among other things, got rid of the horrendous "swizzle sendEvent:" code that caused us so many problems in 1.9.0, so even if that were an option we wouldn't want that.

I'm happy to take this workaound back out if it's fixed on 1.9.2, but given how hard it was to get anything that could affect Firefox but didn't fix a Firefox bug landed once 1.9.0 was "stable", even when it was a serious regression for embedding, I'm not holding my breath. We certainly don't want to make 2.1a1 contingent on that.
(In reply to comment #7)
> So what things are available at the point that you call Activate?
> 
> The DOMWindow, document and content nodes, but no presshell yet?

The DOMWindow and document are available, the presShell isn't. I'm not sure what specifically you mean by content nodes (aContent is always null when called from WindowRaised, if that's what you are referring to).

> Was there no presshell in earlier versions (pre-focus-manager) when you called
> Activate as well? Presumably, no frames were available either. In what way was
> the focus updated in gecko when the presshell/frames were eventually available?

I've never needed to dig into the internals of 1.9.0 focus, so I have no idea.
Landed http://hg.mozilla.org/camino/rev/ce88f8c24e83

Closing since the problematic behavior in Camino is now gone. Olli or Neil, if this is in fact a core bug please file a bug against the appropriate component; I don't know enough about the focus system to be able to file a very precise bug.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: camino2.1a1? → camino2.1a1+
Depends on: 600187
It appears this workaround caused bug 600187. We *really* need a proper Gecko fix for this :-\

cl
You need to log in before you can comment on or make changes to this bug.