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)
Core Graveyard
Embedding: APIs
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)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
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>
Comment 2•24 years ago
|
||
I can confirm this bug on both TestGtkEmbed and galeon.
Can it be fixed for 0.9 ? It makes embedders unusable ... THX :)
Comment 4•24 years ago
|
||
saari's got some focus changes he's going to be checking in.
Cc:ing him
Assignee | ||
Comment 5•24 years ago
|
||
This is happening on Mac (PPEmbed) too. Started happening only a few days ago.
Assignee | ||
Comment 6•24 years 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.
Comment 7•24 years ago
|
||
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.
Assignee | ||
Comment 8•24 years ago
|
||
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
Assignee | ||
Comment 9•24 years ago
|
||
Assignee | ||
Comment 10•24 years ago
|
||
Also, winEmbed suffers the same problem.
Comment 11•24 years ago
|
||
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?
Comment 12•24 years ago
|
||
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....
Comment 13•24 years ago
|
||
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.
Updated•24 years ago
|
Whiteboard: critical for mozilla 0.9
Comment 14•24 years ago
|
||
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
Comment 15•24 years ago
|
||
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.
Assignee | ||
Comment 16•24 years ago
|
||
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.
Comment 17•24 years ago
|
||
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.
Assignee | ||
Comment 18•24 years ago
|
||
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.
Comment 19•24 years ago
|
||
I never had that problem in gtk since the code in widget/src/gtk would handle it
for me.
Comment 20•24 years ago
|
||
Comment 21•24 years ago
|
||
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.
Comment 22•24 years ago
|
||
Comment 23•24 years ago
|
||
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.
Assignee | ||
Comment 24•24 years ago
|
||
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.
Updated•24 years ago
|
Whiteboard: critical for mozilla 0.9 → critical for mozilla 0.9, have patch, need r=,sr=,a=
Comment 25•24 years ago
|
||
Comment 26•24 years ago
|
||
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.
Comment 27•24 years ago
|
||
Comment 28•24 years ago
|
||
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.
Comment 29•24 years ago
|
||
Oh, this fixes 71224, too.
Assignee | ||
Comment 30•24 years ago
|
||
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?
Comment 31•24 years ago
|
||
r=sfraser on attachment 31760 [details] [diff] [review]
Updated•24 years ago
|
QA Contact: mdunn → depstein
Comment 32•24 years ago
|
||
sr=tor on attachment 32538 [details] [diff] [review]
Comment 33•24 years ago
|
||
r=bryner on attachment 32538 [details] [diff] [review]
Comment 34•24 years ago
|
||
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.
Comment 35•24 years ago
|
||
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
Comment 36•24 years ago
|
||
*** Bug 78610 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 37•24 years ago
|
||
CC'ing scc for sr of attachment 31760 [details] [diff] [review].
Status: NEW → ASSIGNED
Comment 38•24 years ago
|
||
Can we get this in 0.9?
Comment 39•24 years ago
|
||
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
Comment 41•24 years ago
|
||
Upped severity
Assignee | ||
Comment 42•24 years ago
|
||
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=.
Comment 43•24 years ago
|
||
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.
Comment 44•24 years ago
|
||
Well, it works in mfcembed, but doesn't work in WinEmbed or Webclient. Can
someone please tell me why?
Comment 45•24 years ago
|
||
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;
}
}
Comment 46•24 years ago
|
||
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().
Assignee | ||
Comment 47•24 years ago
|
||
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.
Assignee | ||
Comment 48•24 years ago
|
||
Ed, I should've mentioned this but see the discussion on bug 78514. The patches
here may turn out to be a temporary measure.
Assignee | ||
Comment 49•24 years ago
|
||
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
Comment 50•24 years ago
|
||
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.
Comment 51•23 years ago
|
||
verified code checkin for CBrowserShell.cpp, nsWindow.cpp,EmbedPrivate.cpp,
gtkmozembed2.cpp, & gtkmozarea.c. changes are in local mozilla build.
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•