Closed Bug 78514 Opened 19 years ago Closed 19 years ago

embedded gecko will not relinquish focus to a different window

Categories

(Core Graveyard :: Embedding: APIs, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: DebugWeyers, Assigned: saari)

References

Details

(Whiteboard: needed by 05/16/01)

Attachments

(2 files)

Steps to reproduce:

A gecko window is embedded in a dialog window.  
The gecko window has the focus.  
Tab through until an anchor dom node within the gecko window has the focus.
When a specified anchor node (the last one in the DOM) has the focus, we 
attempt to reset the focus to a control in the parent dialog window by 
calling ::SetFocus(hWnd) where hWnd is the handle to a dialog control.

Expected result: the embedded window loses focus to the dialog control.
Actual result: the embedded window (and currently focused DOM node) maintains 
keyboard focus. 


I'm having difficulty unravelling the focus, unfocus, and blur messages to 
discern how/why our call to SetFocus() seems to succeed and yet gecko maintains 
keyboard focus.
Cc'ing blizzard.
We're doing work in this area to make what you are talking about work properly.
 The embeddor ( in this case your code ) will have to implement an interface
that will listen for the fall off the end of the DOM tree and your app will have
to grab focus.

cc'ing saari + ccarlen since they might have more of an update on where they are
on this.
That's what the nsIWebBrowserChromeFocus interface is for. Your chrome should
implement this iface and it will be called when the user tabs beyond the
contents' last element or shift-tabs before the first one. CC'ing dr because I
think he's responsible for this iface and can explain better. Dan, when your
nsIWebBrowserChromeFocus::Focus*Element is called, do you need to explictly
unfocus the webBrowser by calling nsIWebBrowserFocus::Deactivate()? Or is that
handled internally and you just have to focus on your element? 
> Dan, when your nsIWebBrowserChromeFocus::Focus*Element is called,
> do you need to explictly unfocus the webBrowser by calling
> nsIWebBrowserFocus::Deactivate()? Or is that handled internally
> and you just have to focus on your element?

Hm. I don't actually know how that's supposed to work. Offhand, looking at
nsDocShell::FocusAvailable, it seems like I ought to have set the out-param
|aTookFocus| as we do in the non-embedded case. That might very well be my fault.

If fixing that (i.e., if Focus[Next/Prev]Element succeeds, aTookFocus = true)
doesn't work, I don't really have a clue. It seems like an explicit call to
nsIWebBrowserFocus::Deactivate, either from our nsDocShell code or from the
embeddor's focus code, would be a hack; so I doubt that would be the fix.

Anyway, try the aTookFocus fix and see what happens... I'm confirming this bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
You could call Deactivate if you want, but that isn't really what it was
intended for, it was meant for notifiying the embedded browser when the top
level window is active or not (at the user level). The embedding application
needs to decide where to put focus when the user tabs out of the web browser in
either direction. So, if you need native level focus on your foopy widget in the
embedding client, you're going to have to set it since gecko doesn't know
anything about it, and thus will keep any native concept of focus until you take
it away. It is just notifying you, via nsIWebBrowserChromeFocus, that you
probably want to take focus.

Activate/Deactivate really just unlock/lock the focus controller's memory and
sets its active state (which as you know is being checked by textfields and
others). So if you have focus somewhere in gecko, go to another application
(causing a deactivate of the gecko window and embedding client) and then come
back, gecko will repond to the Activate call by placing focus where it last was.
Due to interest from the very important embedding customer I'm setting --- or
Future target milestone to 0.9.1
Target Milestone: --- → mozilla0.9.1
-> saari
Assignee: adamlock → saari
Blocks: 75642
Okay, so I think this is a hole in the nsIWebBrowserFocus API. The short answer 
is, for now, when you focus something other than the embedded browser, call 
nsIWebBrowserFocus::Deactivate, and reactivate it later when it gets clicked in 
or tabbed into again.

The long answer is that nsIWebBrowserFocus needs Blur and Focus methods in 
addition to Activate and Deactivate. Activate and Deactivate are intended for 
when the user level window of the embedding app is activated or deactivated. 
However, today we don't do anything really special with Activate and Deactivate 
(from the embeddor's perspective anyway), so they can be used interchangably. 
In the future, Activate will grey out controls on MacOS an other platforms as 
desired. That would be incorrect when the user level window still was active, 
and in this case you would call Blur instead.

I wish there was a more automatic way to do this, but I can't think of a way 
given our convoluted XP focus system. The problem boils down to that we don't 
process the blurring of anything until we get the following focus event. The 
reason for this is that a control may not be able to accept focus at all, and 
so we don't want to blur the previous item until we can verify focus acceptance 
by the thing just clicked on... which we don't know about unil the focus event 
comes in.
Status: NEW → ASSIGNED
Don't do anything with patching up the embedding clients with Deactivate just 
yet. I'm brainstorming with a bunch of people about making things better.
I think adding Focus/Blur to nsIWebBrowserFocus is the perfect answer. I am
currently using Activate/Deactivate because, at this point, it does exactly what
I would expect from Focus/Blur. Like you say, Activate/Deactivate may do more,
such as dimming/enabling of controls. I can handle calling the two pairs of
methods at the right time.
Conrad, the short answer is; that works fine for mac, but windows isn't soo pretty. 
And neither is linux.  Wow, did I just add an ugly hack to linux.

I think that some platform experts need to sit down and figure out what to do
here.  I can represent linux/unix/gtk/X and we need some people who are intimate
with win32 and the mac.  Also, I think that saari, joki and hyatt should be
there to talk about what gecko and JS needs from the platform folks.  We can
design a good XP interface from the widget side that way.

We can also design interfaces for getting focus in and out for embedding as well
and maybe even ( gasp! ) make mozilla-the-browser just another user of those
interfaces.

I think the best answer is to stop patching what we have, let the stack of cards
fall and rebuild the focus system from the bottom up with embedding and gecko's
needs from the start.  How do people feel about that?
blizzard, that is, unfortunately, the conclusion I'm coming to. I talked with
hyatt, danm, and joki about this a bit already. Hyatt should be there, but I can
pretty much represent mac and win. Let's do this ASAP to get everyone on the
same page and all issues out.
> that works fine for mac, but windows isn't soo pretty

I don't see what the problem is on Windows - in fact, the child window which
holds the embedded browser will get WM_SETFOCUS/WM_KILLFOCUS messages as well as
WM_ACTIVATE messages. Being able to forward those calls into nsIWebBrowserFocus
would be straightforward.

So, I think that the API of nsIWebBrowserFocus should have Focus and Blur
methods and, as long as the implementation is not interracting with the native
focus in some unfortunate way, it would be fine.
/me is scared shitless.
Conrad, that might work if we changed nsWindow.cpp to always call the DefProc
when WM_MOUSEACTIVATE was recieved so it bounced up the native windows parent chain.

But then you have unparented dialogs, and I'm not sure how they work. Gotta talk
that over with danm.
Blocks: 75664
Whiteboard: needed by 05/13/01
Whiteboard: needed by 05/13/01 → needed by 05/16/01
sr=hyatt
<tabs>
+        if (gLastFocusedContent) {
+          gLastFocusedContent->GetDocument(*getter_AddRefs(doc));
+                     if(doc)
+            doc->GetScriptGlobalObject(getter_AddRefs(ourGlobal));
+          else {
+                       
mDocument->GetScriptGlobalObject(getter_AddRefs(ourGlobal));
+                       NS_RELEASE(gLastFocusedContent);
</tabs>
Are you freeing something that's assured to be null??

+        char className[19];
+        ::GetClassName((HWND)wParam, className, 19);
is 19 (safe) magic?
I was trying to test this patch for myself on a W2K system and ended up with an 
unhandled exception when tabbing off the last link in an embedded window - 

line 2537 of nsDocShell.cpp (nsDocShell::FocusAvailable()) calls the following -
 
nsCOMPtr<nsIWebBrowserChromeFocus> chromeFocus(do_GetInterface(mTreeOwner, 
&ret));

which brings us to Line 117 of nsDocShellTreeOwner.cpp 
(nsDocShellTreeOwner::GetInterface()) -
return mOwnerWin->QueryInterface(aIID, aSink);

where, unfortunately, mOwnerWin is NULL when a QueryInterface call is made upon 
it.


I know you guys are too busy to be debugging my code, but any ideas on why 
mOwnerWin would be null at this point?  I just need a nudge in the right 
direction.
Sorry, it was a minor issue with implementing the nsIEmbeddingSiteWindow 
interface.  

The patch works for me.  
okay, checked in
marking fixed
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Clean up verification of dated code change bus
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.