Closed Bug 406837 Opened 12 years ago Closed 10 years ago

clicking on text input box or address bar doesn't raise virtual keyboard on n800

Categories

(Core :: Widget, defect)

Other
Maemo
defect
Not set

Tracking

()

RESOLVED FIXED
Future
Tracking Status
fennec 1.0+ ---

People

(Reporter: blassey, Assigned: blassey)

References

Details

(Keywords: mobile)

Attachments

(3 files, 8 obsolete files)

33.97 KB, patch
Details | Diff | Splinter Review
1.07 KB, patch
Details | Diff | Splinter Review
1.56 KB, patch
roc
: review+
Details | Diff | Splinter Review
softkb needs to be moved out of minimo and into extensions
This requires the HILDON to be defined in the build system. I'm having trouble making that part work.
Attachment #291490 - Attachment is patch: true
Attachment #291490 - Attachment mime type: application/octet-stream → text/plain
Status: NEW → ASSIGNED
for this to work, add HILDON=1 to your mozconfig.

as discussed on #mobile, we may want something along the lines of --enable-platform-support=hildon or something else more generic
Attachment #291490 - Flags: review?(dougt)
Attachment #291491 - Flags: review?(benjamin)
Attachment #291511 - Flags: review?(benjamin)
Comment on attachment 291490 [details] [diff] [review]
fixes soft keyboard extension to work in 1.9

i am not sure why you needed to do this:

-L../../xpcom/string/src/

in the Makefile.in

the ws is a bit off.
Attachment #291490 - Flags: review?(dougt) → review-
Comment on attachment 291491 [details] [diff] [review]
subscribes to software-keyboard and raises the keyboard on the event

>Index: Makefile.in

>+ifdef HILDON
>+DEFINES		+= -DHILDON
>+INCLUDES	+= -I/usr/include/gtk-2.0/gtk	
>+endif

The INCLUDES change is incorrect. MOZ_GTK2_CFLAGS sets -I/usr/include/gtk-2.0 you should be including any subfiles using

#include <gtk/someheader.h>

Otherwise, this looks fine to me but I'm not a widget peer... please ask roc for r+
Attachment #291491 - Flags: review?(benjamin) → review-
Comment on attachment 291511 [details] [diff] [review]
adds HILDON flag to the build system

By long convention, makefile variables in mozilla start with the NS_ prefix so that random environment variables don't affect the build by accident.
Attachment #291511 - Flags: review?(benjamin) → review-
Comment on attachment 291511 [details] [diff] [review]
adds HILDON flag to the build system

couldn't this be something like

NS_PLATFORM_GUI=HILDON  or something like that?
Blocks: n800
Attached patch updated to address comments (obsolete) — Splinter Review
Attachment #291490 - Attachment is obsolete: true
Attachment #291491 - Attachment is obsolete: true
Attachment #291511 - Attachment is obsolete: true
Attachment #315975 - Flags: review?
Attachment #315975 - Flags: review? → review?(roc)
Comment on attachment 315975 [details] [diff] [review]
updated to address comments

spacing is off in:
config/autoconf.mk.in

I am sure you don't want the printf()

use strncmp instead of strcmp()

Do we really have to have this code in widget instead of just living in a component similar to what we did with wince?

also, is NS_HILDON what other applications are using?
+++ minimo/components/softkb/Makefile.in	21 Dec 2007 15:54:52 -0000
@@ -35,7 +35,7 @@
 # ***** END LICENSE BLOCK *****
 
 
-DEPTH		= ../../..
+DEPTH		= ../..

I do not understand this.

+		        -lxpcomglue_s   \
+		        -lstring_s      \
+		        -L../../xpcom/string/src/ \

Or this, why can't you use REQUIRES?

@@ -1,4 +1,4 @@
-dnl -*- Mode: Autoconf; tab-width: 4; indent-tabs-mode: nil; -*-
+

And why this?

+    printf("observed: %s\n",aTopic);

Lose this

+        if(NS_LITERAL_STRING("close").Equals(aData)){

Use EqualsLiteral

+    int count = 0;

What's this for?

+    if(!gContext){
+        GdkWindow* winGdk = GDK_WINDOW(topLevelParent);
+        if(winGdk){
+            gContext = gtk_im_multicontext_new ();
+            gtk_im_context_set_client_window (gContext,winGdk);
+            nsCOMPtr<nsIObserverService> observerService = do_GetService("@mozilla.org/observer-service;1");
+            if (observerService)
+                observerService->AddObserver(this, "software-keyboard",PR_FALSE );

Is adding this to every single Gtk widget actually necessary?

> use strncmp instead of strcmp()

Why?
(In reply to comment #10)
> +++ minimo/components/softkb/Makefile.in        21 Dec 2007 15:54:52 -0000
> @@ -35,7 +35,7 @@
>  # ***** END LICENSE BLOCK *****
> 
> 
> -DEPTH          = ../../..
> +DEPTH          = ../..
> 
> I do not understand this.
> 


This depends on bug #405716 to move minimo/components/* to extensions/*.  I'd assume that change would be made when they are moved


> +                       -lxpcomglue_s   \
> +                       -lstring_s      \
> +                       -L../../xpcom/string/src/ \
> 
> Or this, why can't you use REQUIRES?
>
The minimo component was written using internal string classes; once moved to extensions it gets linked against external string classes.  Rewritting it to use external interfaces got messy, is there a way to specify internal linkage with REQUIRES?
 
> @@ -1,4 +1,4 @@
> -dnl -*- Mode: Autoconf; tab-width: 4; indent-tabs-mode: nil; -*-
> +
> 
> And why this?

I'm assuming the mode line was added after I moved the code from minimo/components to extensions and I bashed it when I moved it back for the diff.  I'll put it back in.

> 
> +    printf("observed: %s\n",aTopic);
> 
> Lose this
> 

sure thing
> +        if(NS_LITERAL_STRING("close").Equals(aData)){
> 
> Use EqualsLiteral
> 

sure thing

> +    int count = 0;
> 
> What's this for?
>
debugging cruft, I'll remove it
 
> +    if(!gContext){
> +        GdkWindow* winGdk = GDK_WINDOW(topLevelParent);
> +        if(winGdk){
> +            gContext = gtk_im_multicontext_new ();
> +            gtk_im_context_set_client_window (gContext,winGdk);
> +            nsCOMPtr<nsIObserverService> observerService =
> do_GetService("@mozilla.org/observer-service;1");
> +            if (observerService)
> +                observerService->AddObserver(this,
> "software-keyboard",PR_FALSE );
> 
> Is adding this to every single Gtk widget actually necessary?
> 

My intention was to add it to all of the windows, is that not what I'm doing?

> > use strncmp instead of strcmp()
> 
> Why?
> 

Depends on: 405716
You're adding it to all child widgets --- but not toplevel windows --- as far as I can tell.

Either you're internal or you're external. If you're internal you should be able to use REQUIRES. If you're external you shouldn't be taking shortcuts and trying to use internal interfaces. Up to you.

my previous explanation for the EXTRA_DSO_LIBS turns out to be bogus. We are linking as internal.  However, several symbols are missing without that line.  Would this make you feel more comfortable?


  EXTRA_DSO_LDOPTS += $(MOZ_COMPONENT_LIBS) \
+        $(DIST)/lib/$(LIB_PREFIX)xpcomglue_s.$(LIB_SUFFIX) \
+        $(DEPTH)/xpcom/string/src/$(LIB_PREFIX)string_s.$(LIB_SUFFIX) \
         $(NULL)

the first line is used in several make files, you can see here:
http://mxr.mozilla.org/seamonkey/search?string=xpcomglue&find=Makefile.in&findi=&filter=&hitlimit=&tree=seamonkey

the second line is inspired by :
http://mxr.mozilla.org/seamonkey/source/embedding/browser/activex/src/plugin/Makefile.in#212

which is admittedly not a very good source. Also, as it turns out string_s isn't copied to the dist dir at all.  If anyone can tell me the right thing to put in REQUIRES to avoid either of those lines, I'd be happy to use it.
I'm not an expert, bsmedberg should be able to tell us how to use REQUIRES or explain why REQUIRES isn't wanted here.
Several months before I have created this component for microb browser (maemo).

The code structure is pretty straight forward.  If you, guys, have any questions, feel free to ask.
 
I hope it could be useful to fix this bug.
patches attached to this bug as are will fatally break bug 426344 (gtk focus problems with N800 soft keyboard on OS2008), although they are in the right path. I explain: a "gtkimcontext" object already exists for each (gtk)window (see widget/src/gtk2/nsWindow.cpp at http://lxr.mozilla.org/seamonkey/source/widget/src/gtk2/nsWindow.cpp#5504). Creating a new one from vkb extension can can be problematic then. It is actually possible grab this object with some tweaks to widget side and then from vkb source side:

ENGINE:
(...)
        GtkIMContext *im = IMEGetContext();
+
+        if (mContainer)
+            g_object_set_data(G_OBJECT(mContainer), "imcontext", im);

VKB EXTENSION:
(..)
#ifndef WINCE
GtkIMContext*
nsSoftKeyBoard::GetIMContext () {

  // getting the right gdk_window object needed by gtk_im_* stuff.
  nsCOMPtr<nsIScriptGlobalObject> parentTop = do_QueryInterface(mTopWindow);
  if (!parentTop) return nsnull;

  nsIDocShell *docShell = parentTop->GetDocShell();
  if (!docShell) return nsnull;

  nsCOMPtr<nsIBaseWindow> baseWin(do_QueryInterface(docShell));
  if (!baseWin) return nsnull;

  // get the root origin for this content window
  nsCOMPtr<nsIWidget> mainWidget;
  baseWin->GetMainWidget(getter_AddRefs(mainWidget));

  mGdkWindow = NS_STATIC_CAST(GdkWindow *, mainWidget->GetNativeData(NS_NATIVE_WINDOW));

  if (!mGdkWindow) return nsnull;

    GList *children, *child;
    children = gdk_window_peek_children(mGdkWindow);

    for (child = children; child != NULL; child = child->next)
    {
        gpointer user_data = NULL;
        gdk_window_get_user_data(GDK_WINDOW(child->data), &user_data);
        if (user_data)
        {
            gpointer data = NULL;
            data = g_object_get_data(G_OBJECT(user_data), "imcontext");
            if (data) {
              return (GtkIMContext *) data;
            }
        }
    }
    return nsnull;
}
#endif
(..)

doug, blizzard do you think it is reasonable to get checked in ? 

taking the bug ...
Assignee: nobody → tonikitoo
Status: ASSIGNED → NEW
Blocks: 426344
maybe previous patches were tweaking the widget/ too much. I updated it according to doug's comment #9

issues: 

1) if patch is OK I can recreate it changing HILDON by NS_HILDON or even trying to add support to something like "--enable-platform-support=hildon" .. what do you think ?

2) it seems like current panning implementation does not let 'content' to trigger DOM events properly (e.g. 'blur') or even to content to get focused. Btw, that kinda breaks softkb behavior with mine and blassey's patch. mfinkle ?

to apply:
$ cd mozilla
$ patch -p1 < ../<path-to-patch>
Attachment #315975 - Attachment is obsolete: true
Attachment #321674 - Flags: superreview?
Attachment #321674 - Flags: review?
Attachment #315975 - Flags: review?(roc)
You should request review from someone specific.

Would it make more sense to define a method in nsGTKToolkit, which is basically global, to get the IM context you want?
Depends on: 435446
Assignee: tonikitoo → blassey
Attachment #319978 - Attachment is obsolete: true
Attachment #321674 - Attachment is obsolete: true
Attachment #321961 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #330002 - Flags: review?(gavin.sharp)
Attachment #321674 - Flags: superreview?
Attachment #321674 - Flags: review?
Attachment #330003 - Flags: review?(roc) → review?(pavlov)
Comment on attachment 330002 [details] [diff] [review]
slightly updated and moved to a fennec component

I'm not really the right person to review this, not sure who is.
Attachment #330002 - Flags: review?(gavin.sharp)
Duplicate of this bug: 426344
Blocks: 439053
Attached patch IME approach (obsolete) — Splinter Review
This patch has two issues with fennec.  In the address bar, each key press steals focus, which closes the awesome bar until the next key press.  The end effect is that every other key press is lost.  

Also, clicking on a text box rendered to the canvas has no effect.  We might have to do something in the fennec chrome to get around that.
how does the focus-stealing happen? the soft keyboard steals it?
the autocomplete pop up is stealing it, but not completely.  The caret is still in the url entry bar, but the input doesn't go to the bar.
Depends on: 435885
Component: Embedding: GTK Widget → Widget
Flags: blocking1.9.1?
Duplicate of this bug: 437305
Flags: blocking1.9.1? → blocking1.9.1+
Attachment #335015 - Attachment is obsolete: true
Attachment #337616 - Flags: review?(roc)
Depends on: 454235
author	Brad Lassey <blassey@mozilla.com>
	Wed Sep 10 12:12:58 2008 -0400 (at Wed Sep 10 12:12:58 2008 -0400)
changeset 19111	78f592a0a109
parent 19110	0230a5162f93
child 19112	de4f208c9413
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
reopening ....

reproducible w/ fennec 0.9pre from http://people.mozilla.com/~pavlov/fennec/hourly
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
neil's patch in bug 455891 reverts patch bug 454235 changes, and seems to fix this vkb problem.

dep set.
Depends on: 455891
Flags: blocking1.9.1+ → blocking1.9.1-
Attachment #330003 - Flags: review?(pavlov)
tracking-fennec: --- → 1.0b1+
works fine w/ 

xr 85b9917cf2e3
m-b 121ad0a9d105

vkb popsup/hides according to focus in/out event on both content and chrome
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
I am having trouble with this specific issue on build 20090617 (from the nightly drop) when using my nokia n810.

I have tried in regular app mode as well as full screen.  The soft keyboard never pops up when clicking on the URL bar or other text input fields.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #33)
> I am having trouble with this specific issue on build 20090617 (from the
> nightly drop) when using my nokia n810.
> 
> I have tried in regular app mode as well as full screen.  The soft keyboard
> never pops up when clicking on the URL bar or other text input fields.

joel I am assuming you have set you IM to popup vkb in n810.

-> control panel -> text input settings -> on screen -> enable stylus input method
yes, this is checked, thanks for double checking.  This has worked before in the past, it is just odd that it stopped working all the sudden.
Joel - If you load google.com, does the textbox on the page get focus? (cursor and yellow background)
mfinkle, yes it does.  for some reason that brings up the soft keyboard, but youtube does not.  Both sites have focus (including yellow background) on the search box.

I have not seen the soft keyboard for the url bar on any site.
Is this a trunk build?  If so, the recent focus rewrite could be to blame -- I've seen some problems on Windows CE, being unable to type in input boxes in dialogs (e.g. prompt()).
this is built from trunk (xulrunner-1.9.2a1pre builds)
tracking-fennec: 1.0b1+ → 1.0+
(In reply to comment #37)
> mfinkle, yes it does.  for some reason that brings up the soft keyboard, but
> youtube does not.  Both sites have focus (including yellow background) on the
> search box.
perhaps you're seeing what's being described in bug 511975. If that is the case, let's re-close this bug and concentrate on this particular issue in that bug.
QA Contact: gtk-widget → general
Status: REOPENED → RESOLVED
Closed: 11 years ago10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.