Closed Bug 76617 Opened 24 years ago Closed 24 years ago

some embedding apps can not get input focus

Categories

(Core Graveyard :: Embedding: APIs, defect)

defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9

People

(Reporter: masaki.katakai, Assigned: ccarlen)

References

Details

(Whiteboard: critical for mozilla 0.9, have patch, need r=,sr=,a=)

Attachments

(5 files)

I found gtkEmbed of nightly build has a problem that INPUT can not get any input focus. I could not enter any characters, no cursor appeared. Try the following simple HTML, start gtkEmbed and click on INPUT and TEXTAREA. Only INPUT field has problem. I can give a focus to TEXTAREA. <HTML> <TITLE> FORM TEST </TITLE> <BODY> <FORM> <INPUT TYPE="text" NAME="TESTMSG1" SIZE=40> </FORM> <FORM> <TEXTAREA NAME="TEST_TEXTAREA" ROWS=3 COLS=40></TEXTAREA> </FORM> </BODY> </HTML>
Reassigning
Assignee: adamlock → blizzard
I can confirm this bug on both TestGtkEmbed and galeon. Can it be fixed for 0.9 ? It makes embedders unusable ... THX :)
Argh! Bad bug, bad bug!
Status: NEW → ASSIGNED
saari's got some focus changes he's going to be checking in. Cc:ing him
This is happening on Mac (PPEmbed) too. Started happening only a few days ago.
I've seen this: When the root focus controlller is found for a text input element, here: http://lxr.mozilla.org/seamonkey/source/content/html/content/src/nsHTMLInputElement.cpp#695, the test of IsActive is always FALSE with an embedded app. With Seamonkey, it's active when you would expect.
Have a look to make sure that the NS_ACTIVATE event is making it down to the event state controller. It should be. If it's not then we've got a problem.
Alright, found the problem. Neither gtkEmbed, gtkWidget, or PPEmbed call nsIWebBrowserFocus::Activate or Deactivate. Now, that's nescesary. Damn it, it would have been nice to have gotten a heads-up on embedding@netscape.com about nsIWebBrowserFocus coming on-line and that unless a small addition was made to your embedding app, it was going to become #@*#! useless. Here's all it takes to fix PPEmbed. I need to get this into 0.9.
Summary: gtkEmbed: INPUT can not get input focus → some embedding apps can not get input focus
Attached patch Fix for PPEmbed (deleted) — Splinter Review
Also, winEmbed suffers the same problem.
This is really strange because when the webBrowser gets an NS_ACTIVATE event it's supposed to call ::Activate(). I wonder what else has broken?
OK, something really bad has changed here and I'm not sure what it is. The way that the focus would be grabbed in the embedding widget would look something like: o User clicks on the text area o Layout calls |nsIWidget::SetFocus| on the text area's native widget o in |SetFocus| we would grab focus to the toplevel mozilla widget and set the virtual focus inside of the mozilla window heirachy to that window o send an NS_GOTFOCUS and NS_ACTIVATE signal to the layout engine o unwind and it worked. Now when a user clicks on the text area the layout engine never calls SetFocus. From looking at the code in |nsWebBrowser::Activate| it appears that we have to call nsIFocusController::Activate(PR_TRUE) on the focus controller? I guess I can do this when the widget is initialized before there are any focus changes to make it "active." The problem here is that I don't want to unconditionally give focus to the mozilla window until a user does something that makes mozilla ask for focus and it appears that I have to do that given the way that the focus controller works. I'm going to play with this more....
OK, I've got a patch now that calls the Activate() method of the focus controller and it actually does get focus to the window correctly. However, as soon as you put focus somewhere else in the application or give focus to another application you can't get focus back into the window presumably because you have to call the Activate() method on the focus controller again. We need to be able to get focus onto an embedded mozilla window even when the window isn't activated and I bet that the focus controller or the code using it in nsEventStateManager isn't allowing focus to be gived to a window when it isn't active. hyatt? I remember focus checkins from you a few days ago. Does this sound like the intended behaviour or not? If you need to talk about this, let me know.
Whiteboard: critical for mozilla 0.9
setting TM to 0.9 (please forgive the trespass chris.) trying to get the 0.9 lists to make some sense.
Target Milestone: --- → mozilla0.9
I guess this regression was caused by my fix for the textbox focus bug (one of the top annoyances in SeaMonkey and all the embedding clients). In order to prevent windows from coming forward when they shouldn't, a textbox will not allow itself to be focused (using the Focus() method) unless the window it is in is active. I had no idea that some embedding clients didn't respond to this message. MFCEmbed worked fine with the patch. All you have to do is ensure that the Activate method of nsWebBrowser is called when your window is activated. I assumed (seeing as how the method is implemented and called by MFCEMbed) that the embedding clients all supported this method. I guess I was wrong. Anyway, it has always been the case that you needed to call this method when the window gets activated, or focus would have been horked in other ways for you anyway.
Can I get some r=/sr= on the patch to PPEmbed? It's really simple. BeTarget/DontBeTarget are PowerPlant methods which can be overridden by any object which can take the focus. These methods are called by the framework at the right time and just forward things along to nsIWebBrowserFocus.
hyatt, maybe I'm confused. Is the activate method related in any way to the NS_ACTIVATE event? It used to be that when someone called SetFocus() on a widget that the NS_ACTIVATE signal would be sent after the NS_GOTFOCUS event. Now that I think about it what you have here is a lot better than what we have had in the past assuming that what you say is true. It's relatively simple ( although ugly as hell ) for me to call activate when the toplevel window is activated although I have to do some testing.
The new scheme is much better than the old. Before, somehow, the focus just got set automatically. The problem was that this can't take native widgets into account. In the old way, it was possible for a text field within mozilla to be focused at the same time as a native widget in the window (two blinking cursors). Now that the focus has to be explicitly set, I can control it from the focus management code my embedding app. The problem of a native widget and a mozilla widget being focused at the same time is gone.
I never had that problem in gtk since the code in widget/src/gtk would handle it for me.
OK, a few comments about this patch that I've uploaded: I've removed the handlers for NS_ACTIVATE and NS_DEACTIVATE since we shouldn't get those on an inner widget and the new way of doing things The gtk code now seems to handle focus correctly but I wasn't able to use the nsWebBrowser |Activate| or |Deactivate| methods for the toplevel window changes because they were doing too many things. There still isn't a clear seperation in that code that distinguishes between the toplevel window being "active" and the widget itself being active. Because of this I handle the cases myself. There is one really suspect part of the gtk code: +void +EmbedPrivate::FocusOut(void) +{ + nsCOMPtr<nsPIDOMWindow> piWin; + GetPIDOMWindow(getter_AddRefs(piWin)); + + if (!piWin) + return; + + piWin->Deactivate(); + + // but the window is still active until the toplevel gets a focus + // out + nsCOMPtr<nsIFocusController> focusController; + piWin->GetRootFocusController(getter_AddRefs(focusController)); + if (focusController) + focusController->SetActive(PR_TRUE); + +} I have to call focusController->SetActive(PR_TRUE) because if I don't then you can't give focus back to the mozilla window after I've put it somewhere in the application. Am I using nsPIDOMWindow->Deactivate incorrectly? The nsIWidget losing focus isn't enough to be able to turn the blinking cursor off in the application.
The new patch just includes an api change so that people who embed mozilla out of process ( read nautilus ) can track toplevel changes since get_toplevel probably won't work the way I expect.
Chris, that turned out to be a lot of work for gtk. How exactly do nsIWebBrowserFocus::Activate/Deactivate do too much? Is that at the root of the awkwardness? I can't really review because I know next to nothing about about gtk. Anyway, I'd really like to get this in - my app is hosed without it. CC'ing more people.
Whiteboard: critical for mozilla 0.9 → critical for mozilla 0.9, have patch, need r=,sr=,a=
Attached patch final patch (deleted) — Splinter Review
This is the big bad nasty final patch that allows you to keep track of the top level window focus changes. The reason that it's so complicated and that it uses Xlib to do it is because of a bug in anything gtk < 1.2.9 ( which most people use ) and because of gtkplug/gtksocket. Conrad, I'm going to look at the Activate/Deactivate code in a second here to see if I can actually use it or not.
Conrad, to answer you question the Activate() and Deactivate() methods in nsWebBrowser have some problems related to when they try to take focus. For example, if you start a lot of www.google.com in an embedded window and then put focus in another window you will end up with a flashing cursor in the text area of the google search page even though the toplevel doesn't have focus. That's my biggest complaint about it.
Oh, this fixes 71224, too.
patch id=32538 has my r= in that the changes to nsWebBrowser.cpp are fine. patch id=31760 needs r=. Simon - since it's PowerPlant, can you r= that one?
QA Contact: mdunn → depstein
Keywords: patch
saari and/or hyatt need to look at these patches. I don't think I'm qualified to review changes to our delicate focus system.
The gtk code is checked in on the tip and on the 0.9 branch. Over to ccarlen since he still has an outstanding patch in this bug.
Assignee: blizzard → ccarlen
Status: ASSIGNED → NEW
*** Bug 78610 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Can we get this in 0.9?
This problem occurrs on Win32 as well. The patches attached do not fix the problem on Win32. Changing OS and Platform to ALL
OS: Linux → All
Hardware: PC → All
Upped severity
Severity: normal → major
Upped severity
Hold on there - I thought it worked in mfcEmbed. The remaining patch here is for the PowerPlant embedding sample will go in when I get sr=.
I just tested with the latest version mfcembed(my bits are in sync with the trunk as of this morning) and it seems to work just fine. My test involved sending a test e-mail message using mfcembed from my yahoo webmail account i.e. i used the mail compose form which has a combination of text and textarea fields.
Well, it works in mfcembed, but doesn't work in WinEmbed or Webclient. Can someone please tell me why?
Ed : Looks like the code to handle the WM_ACTIVATE windows messages are missing in WinEmbed and WebClient. There's the following code in mfcembed/BrowserFrm.cpp to set the focus explicitly: void CBrowserView::Activate(UINT nState, CWnd* pWndOther, BOOL bMinimized) { nsCOMPtr<nsIWebBrowserFocus> focus(do_GetInterface(mWebBrowser)); if(!focus) return; switch(nState) { case WA_ACTIVE: case WA_CLICKACTIVE: focus->Activate(); break; case WA_INACTIVE: focus->Deactivate(); break; default: break; } }
But how does this Activate() get called? By virtue of what interface? This Activate is certainly win only thing, right? The Activate() you picture here is not the same as nsIWebBrowserChrome::Activate().
See: http://lxr.mozilla.org/seamonkey/source/embedding/tests/mfcembed/BrowserView.cpp#977 It's unfortunately different for every embedding app. In the case of mfcEmbed, the main window gets a WM_ACTIVATE message and forwards it along to the BrowserView::Activate() method.
Ed, I should've mentioned this but see the discussion on bug 78514. The patches here may turn out to be a temporary measure.
Remaining patch for PPEmbed checked into trunk. While gtkWidget was critical for the branch, I'm not sure how many are using this code. At this point, marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
I've reopened 78610 since I'm still seeing this in winEmbed and webclient. I feel that, if I didn't need to make any special calls to allow my users to type in forms before, why should I have to do so now.
verified code checkin for CBrowserShell.cpp, nsWindow.cpp,EmbedPrivate.cpp, gtkmozembed2.cpp, & gtkmozarea.c. changes are in local mozilla build.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: