590299 - Virtual keyboard is not invoked for input fields in Qt plugins

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Sudarsana Nagineni]

Attachment: Patch installs event filter at browser side and update IME state when plugin post InputPanel events

Fennec should show VKB when plugin post QEvent::RequestSoftwareInputPanel and hide it on QEvent::CloseSoftwareInputPanel event. At the moment the VKB is not showing when dom.ipc.plugins.enabled pref is true and plugins running in different process.

To fix this issue, I think the request from the plugin process must be transferred to the parent process and post same events from there.

Comment 1

[Oleg Romashin (:romaxa)]

Comment on attachment 468808 [details] [diff] [review]
Patch installs event filter at browser side and update IME state when plugin post InputPanel events


>+#if defined(MOZ_WIDGET_QT)
>+    if (!mIMEGlobal) {
>+        IMEEventListener* listener = new IMEEventListener(this);
>+        if (listener) {
>+            mIMEGlobal = listener;
>+            QInputContext* inputContext = qApp->inputContext();

According to Qt implementation this might return null.... and we should check it for null here

>+            inputContext->installEventFilter(listener);
>+       }
         ^ indent a bit broken...

Comment 2

[Oleg Romashin (:romaxa)]

Doug do you know if anybody else should take a look at this patch? it is touching a bit non-qt part of code.

Comment 3

[Doug Turner (:dougt)]

jst or josh probably should provide feedback

Comment 4

[Oleg Romashin (:romaxa)]

Comment on attachment 468808 [details] [diff] [review]
Patch installs event filter at browser side and update IME state when plugin post InputPanel events

Josh have you heard anything about VKB-request-open/close-API from plugins to browser?

Comment 5

[Josh Aas]

(In reply to comment #4)
> Comment on attachment 468808 [details] [diff] [review]
> Patch installs event filter at browser side and update IME state when plugin
> post InputPanel events
> 
> Josh have you heard anything about VKB-request-open/close-API from plugins to
> browser?

I have not heard anything on this subject and there is no NPAPI support for things like this. We don't want usage of native APIs to be necessary for NPAPI plugin development so that might need to change via the normal NPAPI proposal process.

Comment 6

[Josh Aas]

I think this patch is essentially adding support for syncing the child and the host wrt the native VKB state, I'm fine with that until we can get an actual NPAPI solution up and running.

Comment 7

[Sudarsana Nagineni]

Attachment: fixed comments

Comment 8

[Oleg Romashin (:romaxa)]

Comment on attachment 475871 [details] [diff] [review]
fixed comments

Ok, looks good now

Comment 9

[Oleg Romashin (:romaxa)]

this is having default build changes... and need approval

Comment 10

[Doug Turner (:dougt)]

jim, this look good too you?

Comment 11

[Oleg Romashin (:romaxa)]

Comment on attachment 475871 [details] [diff] [review]
fixed comments

This is failed to compile on Maemo5:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294476963.1294478503.580.gz#err0

Possibly it is better to move IMEEventListener out from header file

Comment 12

[Oleg Romashin (:romaxa)]

Attachment: Updated patch to trunk

Comment 13

[Oleg Romashin (:romaxa)]

Attachment: Plugins IME activation for Qt plugins

Comment 14

[Oleg Romashin (:romaxa)]

Any review on this?

Comment 15

[Josh Aas]

Perhaps Karl Tomlinson would be a better reviewer for these QT plugin patches?

Comment 16

[Oleg Romashin (:romaxa)]

Comment on attachment 553595 [details] [diff] [review]
Plugins IME activation for Qt plugins

Karl could you take a loot at this patch?

Comment 17

[Karl Tomlinson (:karlt)]

Does this already work in other (non-Qt) ports?
If so, is someone able to fill me in on how this works in other ports?

Comment 18

[Oleg Romashin (:romaxa)]

it does not work in other ports right now at all., UI<>Content<>Plugins path enabled only in Maemo6/Harmattan/Qt. and I'm trying to make it works without long process of NPAPI change for Qt port on maemo.

Comment 19

[Karl Tomlinson (:karlt)]

(In reply to Oleg Romashin (:romaxa) from comment #18)
> UI<>Content<>Plugins path
> enabled only in Maemo6/Harmattan/Qt.

Is there anything significant about Content being a separate process from UI?
The same issues exist with Content and UI in the same process, right?

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't think that qt event should overwrite the our IME state. Looks like this patch breaks IME state management.

The IME state should be nsIWidget::IME_STATUS_PLUGIN when plugin has focus. The behavior of widget can depend on widget at that state. Why don't you just open software keyboard?

# As I said in another bug, I still think that nsIWidget::SetIMEEnabled() shouldn't open software keyboard automatically. nsIWidget should have an independent API for opening software keyboard.

Comment 21

[Oleg Romashin (:romaxa)]

(In reply to Masayuki Nakano (Mozilla Japan) from comment #20)
> I don't think that qt event should overwrite the our IME state. Looks like
> this patch breaks IME state management.
not so much, in this case we just need to send nsIWidget::IME_STATUS_PLUGIN when plugin is editable
and nsIWidget::IME_STATUS_DISABLED when plugin is focused but not editable.

> The IME state should be nsIWidget::IME_STATUS_PLUGIN when plugin has focus.
that is not true, because plugin can be not editable, and IME STATUS != disabled does not make any sense here...

> The behavior of widget can depend on widget at that state. Why don't you
> just open software keyboard?

In order to open VKB we should call API on UI process, and for GTK we do that when IME STATUS != disabled

> # As I said in another bug, I still think that nsIWidget::SetIMEEnabled()
> shouldn't open software keyboard automatically.
actually that is happening now on GTK port...
nsIWidget::SetIMEEnabled->nsGtkIMModule::SetInputMode-> invoke soft keyboard

Comment 22

[Oleg Romashin (:romaxa)]

> # As I said in another bug, I still think that nsIWidget::SetIMEEnabled()
> shouldn't open software keyboard automatically. nsIWidget should have an
> independent API for opening software keyboard.
So we should change all existing API and don't invoke VKB in all ports when IME_STATE != disabled?

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Oleg Romashin (:romaxa) from comment #21)
> (In reply to Masayuki Nakano (Mozilla Japan) from comment #20)
> > I don't think that qt event should overwrite the our IME state. Looks like
> > this patch breaks IME state management.
> not so much, in this case we just need to send nsIWidget::IME_STATUS_PLUGIN
> when plugin is editable
> and nsIWidget::IME_STATUS_DISABLED when plugin is focused but not editable.

No. We *cannot* know whether the plugin is editable or not. Therefore, nsIWidget::IME_STATUS_PLUGIN doesn't mean that current focused widget in plugin editable. It means only "a plugin is last focused content in the widget".

Each widget code should behave the best for each platform's design. For example, on Windows, the widget should behave as "enabled".  on GTK, the widget shouldn't use its owning IM content (plugin should handle all IM events directly), see bug 674456.

I mean, if widget's state is nsIWidget::IME_STATUS_PLUGIN, the behavior of widget should be designed on each platform's maintainer.

> > The IME state should be nsIWidget::IME_STATUS_PLUGIN when plugin has focus.
> that is not true, because plugin can be not editable, and IME STATUS !=
> disabled does not make any sense here...

Yes. nsIWidget::IME_STATUS_PLUGIN can be when the focused widget isn't editable. nsIWidget::IME_STATUS_PLUGIN doesn't suggest the actual editable state.

> > The behavior of widget can depend on widget at that state. Why don't you
> > just open software keyboard?
> 
> In order to open VKB we should call API on UI process, and for GTK we do
> that when IME STATUS != disabled

Okay, GTK does it automatically, but qt doesn't do it now, right?

I suggest that you should add new API nsIWidget::OpenSoftwareKeyboard() and nsIWidget::CloseSoftwareKeyboard(), and call them directly rather than calling nsIMEStateManager::UpdateIMEState(). (nsBaseWidget should implement them and just return NS_ERROR_NOT_IMPLEMENTED for other platforms)

> > # As I said in another bug, I still think that nsIWidget::SetIMEEnabled()
> > shouldn't open software keyboard automatically.
> actually that is happening now on GTK port...
> nsIWidget::SetIMEEnabled->nsGtkIMModule::SetInputMode-> invoke soft keyboard

Yes, I suggested that at a bug of GTK widget (forgot the bug#).

(In reply to Oleg Romashin (:romaxa) from comment #22)
> > # As I said in another bug, I still think that nsIWidget::SetIMEEnabled()
> > shouldn't open software keyboard automatically. nsIWidget should have an
> > independent API for opening software keyboard.
> So we should change all existing API and don't invoke VKB in all ports when
> IME_STATE != disabled?

Ideally, I think so, but you don't need to do it in this bug.

Comment 24

[Oleg Romashin (:romaxa)]

what is SetIMEOpenState ? can I use that for triggering VKB open/close?

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Oleg Romashin (:romaxa) from comment #24)
> what is SetIMEOpenState ? can I use that for triggering VKB open/close?

It's used for ime-mode: active/inactive.
https://developer.mozilla.org/en/CSS/ime-mode

Don't use it for this bug because it was designed for changes IME's mode. Turing on/off IME is also said as changing input language between English and another. The purpose is different.

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Masayuki Nakano (Mozilla Japan) from comment #25)
> Turing on/off IME is also said as changing input language between English
> and another. The purpose is different.

I meant opening/closing IME is same as switching the input language between English and another.

Comment 27

[Oleg Romashin (:romaxa)]

Attachment: Plugins IME activation for Qt plugins

Probably some functions could be renamed, but this something that works and hopefully all comments addressed

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 555323 [details] [diff] [review]
Plugins IME activation for Qt plugins

>diff --git a/content/events/src/nsIMEStateManager.cpp b/content/events/src/nsIMEStateManager.cpp
>--- a/content/events/src/nsIMEStateManager.cpp
>+++ b/content/events/src/nsIMEStateManager.cpp
>@@ -263,16 +263,35 @@ nsIMEStateManager::GetNewIMEState(nsPres
>     if (doc && doc->HasFlag(NODE_IS_EDITABLE))
>       return nsIContent::IME_STATUS_ENABLE;
>     return nsIContent::IME_STATUS_DISABLE;
>   }
> 
>   return aContent->GetDesiredIMEState();
> }
> 
>+void
>+nsIMEStateManager::UpdatePluginEditableState(PRBool aEditable, nsIContent* aContent)
>+{
>+  if (!sPresContext) {
>+    NS_WARNING("ISM doesn't know which editor has focus");
>+    return;
>+  }
>+
>+  nsCOMPtr<nsIWidget> widget = GetWidget(sPresContext);
>+  if (!widget) {
>+    NS_WARNING("focused widget is not found");
>+    return;
>+  }
>+
>+  if (NS_FAILED(widget->SetSoftwareKeyboardState(aEditable))) {
>+    return; // This platform doesn't support controling the VKB state.
>+  }
>+}
>+
> // Helper class, used for IME enabled state change notification
> class IMEEnabledStateChangedEvent : public nsRunnable {
> public:
>   IMEEnabledStateChangedEvent(PRUint32 aState)
>     : mState(aState)
>   {
>   }
> 

Hmm, I don't like this method in ISM because this isn't used by other platforms but I have no better idea. So, let's use this approach...

I think the method should be nsIMEStateManager::SetSoftwareKeyboardState(PRBool aOpen). And you don't need to check the last if statement.

>diff --git a/dom/plugins/ipc/Makefile.in b/dom/plugins/ipc/Makefile.in
>--- a/dom/plugins/ipc/Makefile.in
>+++ b/dom/plugins/ipc/Makefile.in
>@@ -150,16 +150,17 @@ CMMSRCS   += \
> EXPORTS_mozilla/plugins += \
>     PluginInterposeOSX.h \
>     $(NULL)
> endif
> 
> LOCAL_INCLUDES = \
>   -I$(srcdir)/../base \
>   -I$(topsrcdir)/xpcom/base/ \
>+  -I$(topsrcdir)/content/events/src \

Is this still needed?

>@@ -89,16 +92,51 @@ const PRUnichar * kMozillaWindowClass = 
> namespace {
> PluginModuleChild* gInstance = nsnull;
> }
> 
> #ifdef MOZ_WIDGET_QT
> typedef void (*_gtk_init_fn)(int argc, char **argv);
> static _gtk_init_fn s_gtk_init = nsnull;
> static PRLibrary *sGtkLib = nsnull;
>+
>+namespace mozilla {
>+namespace plugins {
>+
>+class IMEEventListener : public QObject
>+{
>+public:
>+    IMEEventListener(PluginModuleChild* aPMChild) : mPMChild(aPMChild) {}
>+    ~IMEEventListener() {}
>+
>+protected:
>+    virtual bool eventFilter(QObject* obj, QEvent* event);
>+
>+private:
>+    PluginModuleChild* mPMChild;
>+};
>+
>+} /* namespace plugins */
>+} /* namespace mozilla */
>+
>+bool
>+IMEEventListener::eventFilter(QObject* obj, QEvent* event)
>+{
>+    if (mPMChild) {
>+        if (event->type() == QEvent::RequestSoftwareInputPanel) {
>+            mPMChild->SendSetPluginEditableState(true);
>+        }
>+        else if (event->type() == QEvent::CloseSoftwareInputPanel) {
>+            mPMChild->SendSetPluginEditableState(false);
>+        }
>+    }
>+
>+    // standard event processing
>+    return QObject::eventFilter(obj, event);
>+}
> #endif

Is this event fired only for the focused widget? If this is fired on inactive widget too, this patch breaks the active widget's behavior.

>diff --git a/widget/public/nsIWidget.h b/widget/public/nsIWidget.h
>--- a/widget/public/nsIWidget.h
>+++ b/widget/public/nsIWidget.h
>@@ -1373,16 +1373,21 @@ class nsIWidget : public nsISupports {
>                                PRUint32 aNewEnd) = 0;
> 
>     /*
>      * Selection has changed in the focused node
>      */
>     NS_IMETHOD OnIMESelectionChange(void) = 0;
> 
>     /*
>+     * Open or close VKB
>+     */
>+    NS_IMETHOD SetSoftwareKeyboardState(PRBool aOpened) = 0;
>+
>+    /*
>      * Retrieves preference for IME updates
>      */
>     virtual nsIMEUpdatePreference GetIMEUpdatePreference() = 0;
> 
>     /*
>      * Call this method when a dialog is opened which has a default button.
>      * The button's rectangle should be supplied in aButtonRect.
>      */ 

Shouldn't aOpened be aOpen? I'm not sure the different of the feeling for native speakers.

You need to change uuid. And needs sr for this. I recommend roc.

>diff --git a/widget/src/qt/nsWindow.cpp b/widget/src/qt/nsWindow.cpp
>+NS_IMETHODIMP
>+nsWindow::SetSoftwareKeyboardState(PRBool aOpened)
>+{
>+    PRInt32 openDelay =
>+        Preferences::GetInt("ui.vkb.open.delay", 200);
>+    if (aOpened) {
>+        mWidget->requestVKB(openDelay);
>+    } else {
>+        mWidget->hideVKB();
>+    }
>+    return NS_OK;
>+}
>+

Don't you need to check the IME state here? If this method was accidentally called for open, it could cause serious regression.

Comment 29

[Oleg Romashin (:romaxa)]

> >   -I$(topsrcdir)/xpcom/base/ \
> >+  -I$(topsrcdir)/content/events/src \
> Is this still needed?
yes otherwise we don't see nsIMEStateManager.h include

> Is this event fired only for the focused widget? If this is fired on
> inactive widget too, this patch breaks the active widget's behavior.

No it is fired only when plugin is focused

Comment 30

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

Comment 31

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 555461 [details] [diff] [review]
Plugins VKB activation for Qt plugins

>+void
>+nsIMEStateManager::SetSoftwareKeyboardState(PRBool aOpen, nsIContent* aContent)
>+{

Why do you still keep the second param? I don't think it's needed.

>+  if (!sPresContext) {
>+    NS_WARNING("ISM doesn't know which editor has focus");
>+    return;
>+  }

NS_ENSURE_TRUE(sPresContext, );

>+
>+  nsCOMPtr<nsIWidget> widget = GetWidget(sPresContext);
>+  if (!widget) {
>+    NS_WARNING("focused widget is not found");
>+    return;
>+  }

NS_ENSURE_TRUE(widget, );

rather than if and NS_WARNING. Sorry, I should have pointed in previous review.

>diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp
>--- a/dom/ipc/TabParent.cpp
>+++ b/dom/ipc/TabParent.cpp
>@@ -603,10 +603,19 @@ TabParent::RecvSetIMEOpenState(const PRB
>     widget->SetIMEOpenState(aValue);
>   return true;
> }
> 
> bool
>+TabParent::RecvSetSoftwareKeyboardState(const PRBool& aValue)
>+{
>+  nsCOMPtr<nsIWidget> widget = GetWidget();
>+  if (widget && AllowContentIME())
>+    widget->SetSoftwareKeyboardState(aValue);
>+  return true;
>+}

This isn't related in this bug, but I should comment it. AllowContentIME() causes misleading. It just checks editable state of focused node. It should be renamed IsFocusNodeEditable(). I'll file a bug for this.

>diff --git a/dom/plugins/ipc/PluginModuleChild.cpp b/dom/plugins/ipc/PluginModuleChild.cpp
>--- a/dom/plugins/ipc/PluginModuleChild.cpp
>+++ b/dom/plugins/ipc/PluginModuleChild.cpp
>@@ -66,10 +68,11 @@
> #include "mozilla/plugins/BrowserStreamChild.h"
> #include "mozilla/plugins/PluginStreamChild.h"
> #include "PluginIdentifierChild.h"
> 
> #include "nsNPAPIPlugin.h"
>+#include "nsIContent.h"

Why is this needed?

>diff --git a/widget/src/qt/nsWindow.cpp b/widget/src/qt/nsWindow.cpp
>--- a/widget/src/qt/nsWindow.cpp
>+++ b/widget/src/qt/nsWindow.cpp
>@@ -3091,43 +3091,57 @@ nsWindow::SetInputMode(const IMEContext&
> {
>     NS_ENSURE_TRUE(mWidget, NS_ERROR_FAILURE);
> 
>     mIMEContext = aContext;

I think that you should document a warning comment before this line. This method is going to call SetSoftwareKeyboardState() and it refers mIMEContext. If somebody moved this to next to the switch statement, the behavior would be broken.

>-     // Ensure that opening the virtual keyboard is allowed for this specific
>-     // IMEContext depending on the content.ime.strict.policy pref
>-     if (aContext.mStatus != nsIWidget::IME_STATUS_DISABLED && 
>-         aContext.mStatus != nsIWidget::IME_STATUS_PLUGIN) {
>-       if (Preferences::GetBool("content.ime.strict_policy", PR_FALSE) &&
>-           !aContext.FocusMovedByUser() &&
>-           aContext.FocusMovedInContentProcess()) {
>-         return NS_OK;
>-       }
>-     }
>-
>-    switch (aContext.mStatus) {
>+    switch (mIMEContext.mStatus) {
>         case nsIWidget::IME_STATUS_ENABLED:
>         case nsIWidget::IME_STATUS_PASSWORD:
>         case nsIWidget::IME_STATUS_PLUGIN:
>-            {
>-                PRInt32 openDelay =
>-                    Preferences::GetInt("ui.vkb.open.delay", 200);
>-                mWidget->requestVKB(openDelay);
>-            }
>+            SetSoftwareKeyboardState(true);
>             break;
>         default:
>-            mWidget->hideVKB();
>+            SetSoftwareKeyboardState(false);
>             break;
>     }
> 
>     return NS_OK;
> }
> 
> NS_IMETHODIMP
> nsWindow::GetInputMode(IMEContext& aContext)
> {
>     aContext = mIMEContext;
>+
>+    return NS_OK;
>+}
>+
>+NS_IMETHODIMP
>+nsWindow::SetSoftwareKeyboardState(PRBool aOpen)
>+{
>+    // Ensure that opening the virtual keyboard is allowed for this specific
>+    // IMEContext depending on the content.ime.strict.policy pref
>+    if (mIMEContext.mStatus != nsIWidget::IME_STATUS_DISABLED && 
>+        mIMEContext.mStatus != nsIWidget::IME_STATUS_PLUGIN) {
>+        if (Preferences::GetBool("content.ime.strict_policy", PR_FALSE) &&
>+            !mIMEContext.FocusMovedByUser() &&
>+            mIMEContext.FocusMovedInContentProcess()) {
>+            return NS_OK;
>+        }
>+    }

When aOpen is FALSE, this check isn't needed, no?

>+
>+    if (mIMEContext.mStatus == nsIWidget::IME_STATUS_DISABLED && aOpen) {
>+        return NS_ERROR_UNEXPECTED;
>+    }

I think:

if (aOpen) {
    NS_ENSURE_TRUE(mIMEContext.mStatus != nsIWidget::IME_STATUS_DISABLED,
                   NS_ERROR_UNEXPECTED);

    // Ensure that opening the virtual keyboard is allowed for this specific
    // IMEContext depending on the content.ime.strict.policy pref
    if (mIMEContext.mStatus != nsIWidget::IME_STATUS_PLUGIN &&
        Preferences::GetBool("content.ime.strict_policy", PR_FALSE) &&
        !mIMEContext.FocusMovedByUser() &&
        mIMEContext.FocusMovedInContentProcess()) {
        return NS_OK;
    }
}

>+
>+    PRInt32 openDelay =
>+        Preferences::GetInt("ui.vkb.open.delay", 200);

Move this to inside next if block. else block doesn't need openDelay.

Comment 32

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

More comments fixed

Comment 33

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Masayuki Nakano (Mozilla Japan) from comment #31)
> >diff --git a/widget/src/qt/nsWindow.cpp b/widget/src/qt/nsWindow.cpp
> >--- a/widget/src/qt/nsWindow.cpp
> >+++ b/widget/src/qt/nsWindow.cpp
> >@@ -3091,43 +3091,57 @@ nsWindow::SetInputMode(const IMEContext&
> > {
> >     NS_ENSURE_TRUE(mWidget, NS_ERROR_FAILURE);
> > 
> >     mIMEContext = aContext;
> 
> I think that you should document a warning comment before this line. This
> method is going to call SetSoftwareKeyboardState() and it refers
> mIMEContext. If somebody moved this to next to the switch statement, the
> behavior would be broken.

I'd like you to add some comments about this...

> >+NS_IMETHODIMP
> >+nsWindow::SetSoftwareKeyboardState(PRBool aOpen)
> >+{
> >+    // Ensure that opening the virtual keyboard is allowed for this specific
> >+    // IMEContext depending on the content.ime.strict.policy pref
> >+    if (mIMEContext.mStatus != nsIWidget::IME_STATUS_DISABLED && 
> >+        mIMEContext.mStatus != nsIWidget::IME_STATUS_PLUGIN) {
> >+        if (Preferences::GetBool("content.ime.strict_policy", PR_FALSE) &&
> >+            !mIMEContext.FocusMovedByUser() &&
> >+            mIMEContext.FocusMovedInContentProcess()) {
> >+            return NS_OK;
> >+        }
> >+    }
> 
> When aOpen is FALSE, this check isn't needed, no?
> 
> >+
> >+    if (mIMEContext.mStatus == nsIWidget::IME_STATUS_DISABLED && aOpen) {
> >+        return NS_ERROR_UNEXPECTED;
> >+    }
> 
> I think:
> 
> if (aOpen) {
>     NS_ENSURE_TRUE(mIMEContext.mStatus != nsIWidget::IME_STATUS_DISABLED,
>                    NS_ERROR_UNEXPECTED);
> 
>     // Ensure that opening the virtual keyboard is allowed for this specific
>     // IMEContext depending on the content.ime.strict.policy pref
>     if (mIMEContext.mStatus != nsIWidget::IME_STATUS_PLUGIN &&
>         Preferences::GetBool("content.ime.strict_policy", PR_FALSE) &&
>         !mIMEContext.FocusMovedByUser() &&
>         mIMEContext.FocusMovedInContentProcess()) {
>         return NS_OK;
>     }
> }

How about this? You can move |if (mIMEContext.mStatus == nsIWidget::IME_STATUS_DISABLED && aOpen) {| in |if (aOpen) {| and make simpler.
# |mIMEContext.mStatus != nsIWidget::IME_STATUS_DISABLED &&| can be removed by the order of checking.

Comment 34

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

Comment 35

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 555632 [details] [diff] [review]
Plugins VKB activation for Qt plugins

>+        if (mIMEContext.mStatus != nsIWidget::IME_STATUS_PLUGIN) {
>+            if (Preferences::GetBool("content.ime.strict_policy", PR_FALSE) &&
>+                !mIMEContext.FocusMovedByUser() &&
>+                mIMEContext.FocusMovedInContentProcess()) {
>+                return NS_OK;
>+            }
>+        }

Please use |&&|, you can check these conditions by an |if| statement. Otherwise, looks fine for me. Thank you.

And I'm not familiar with the code under dom/, please request additional review for the module owner or peer if it's necessary.

Comment 36

[Oleg Romashin (:romaxa)]

Comment on attachment 555632 [details] [diff] [review]
Plugins VKB activation for Qt plugins

Olly can you look at dom part ?

Comment 37

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 555632 [details] [diff] [review]
Plugins VKB activation for Qt plugins

Review of attachment 555632 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/events/src/nsIMEStateManager.h
@@ +87,5 @@
>    // widget.  So, the caller must have focus.
>    // aNewIMEState must have an enabled state of nsIContent::IME_STATUS_*.
>    // And optionally, it can have an open state of nsIContent::IME_STATUS_*.
>    static void UpdateIMEState(PRUint32 aNewIMEState, nsIContent* aContent);
> +  static void SetSoftwareKeyboardState(PRBool aOpen);

Please document what this does.

::: dom/ipc/PBrowser.ipdl
@@ +178,5 @@
>      sync GetIMEOpenState() returns (PRBool value);
>  
>      SetIMEOpenState(PRBool value);
>  
> +    SetSoftwareKeyboardState(PRBool value);

And this.

And document why we need to have separate IME-open and keyboard-open states. (I don't know why!)

Comment 38

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> And document why we need to have separate IME-open and keyboard-open states. (I don't know why!)

Opening IME means that it switches input language. For example, when the keyboard setting on Android is Japanese, you can switch input language between ASCII and Japanese by a button on software keyboard.

Comment 39

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

Added documentation for new method

Comment 40

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Masayuki Nakano (Mozilla Japan) from comment #38)
> > And document why we need to have separate IME-open and keyboard-open states. (I don't know why!)
> 
> Opening IME means that it switches input language. For example, when the
> keyboard setting on Android is Japanese, you can switch input language
> between ASCII and Japanese by a button on software keyboard.

I don't understand that explanation. Are you saying that when the keyboard is up and entering ASCII, we think the IME is "closed" but the keyboard is "open"? Can the IME be "open" while the keyboard is "closed"?

Comment 41

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #40)
> (In reply to Masayuki Nakano (Mozilla Japan) from comment #38)
> > > And document why we need to have separate IME-open and keyboard-open states. (I don't know why!)
> > 
> > Opening IME means that it switches input language. For example, when the
> > keyboard setting on Android is Japanese, you can switch input language
> > between ASCII and Japanese by a button on software keyboard.
> 
> I don't understand that explanation. Are you saying that when the keyboard
> is up and entering ASCII, we think the IME is "closed" but the keyboard is
> "open"?

Yes.

> Can the IME be "open" while the keyboard is "closed"?

Yes. Opening/closing software keyboard doesn't change the IME open/closed. If user changed the input mode to Japanese (open) and close the software keyboard and reopen the software keyboard again by tapping in <input>, the input mode should be Japanese.  And also if user changed the input mode to ASCII (closed) and close the software keyboard and reopen the software keyboard again by tapping in <input>, the input mode should be ASCII.

Note that open/close is a behavior on Windows and Linux. On the other platforms, the open mode is just a keyboard layout (an input mode). Therefore, anyway, SetIMEOpenState() doesn't make sense in some platforms.

Comment 42

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't understand why we need to track those things separately. Can't we just say the keyboard is the IME, and say that the IME is open while the keyboard is up and closed when it's not?

Comment 43

[Oleg Romashin (:romaxa)]

That was original proposal, but recommendation was to have separate API in comment 20

Comment 44

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> Can't we just say the keyboard is the IME

No, keyboard and IME are different.

Comment 45

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Software keyboard is a device to input text.

IME is an assistant software for suggesting the result from inputted text.

The action on software keyboard is passed to IME and you can show the suggested words if you type Western language.

Comment 46

[Robert O'Callahan (:roc) (email my personal email if necessary)]

They're different, but why does Gecko have to treat them differently? Why can't we treat the software keyboard as an extended IME?

Is there a problem we can't solve if we treat the keyboard as an IME?

Comment 47

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #46)
> Is there a problem we can't solve if we treat the keyboard as an IME?

As I said, we can switch the input language by software keyboard. Therefore, "close IME" doesn't make sense for me if we try to close software keyboard. I feel that it means that input language is changed to ASCII and the state of software keyboard isn't changing.

Comment 48

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

And also, if we change the behavior of SetIMEOpenState(), it means that it changes the behavior of |ime-mode: active;| and |ime-mode: inactive;|.

Comment 49

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Wouldn't it make more sense to have one bit of state indicating whether the input method UI is open or not, and additional state to indicate what language or character set the input method should be targeting?

Comment 50

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #49)
> Wouldn't it make more sense to have one bit of state indicating whether the
> input method UI is open or not, and additional state to indicate what
> language or character set the input method should be targeting?

Did you mean that the new bit should be in the param of SetInputMode()? If so, I don't think so because SetInputMode() should set only the states which are not changed by users.  On the other hand, if you meant that it should be in the param of SetIMEOpenState(), I think it's okay because both states are changeable state by user. *However*, I think that SetIMEOpenState() should be renamed to SetInputState() or something.

Comment 51

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Actually, I'm sort of confused about why this patch needs to modify nsIMEStateManager. Why don't we just get a widget and call SetSoftwareKeyboardState on it manually? In fact, since this is really platform-specific code, can we solve it in a platform-specific way? For example, can the Qt plugin code send a message to the browser's Qt window that triggers the opening/closing of the virtual keyboard?

Comment 52

[Oleg Romashin (:romaxa)]

That is possible, we can call this code: http://mxr.mozilla.org/mozilla-central/source/widget/src/qt/mozqwidget.cpp#595
in any place in UI process. should I do that?

Comment 53

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes, please give it a go.

Comment 54

[Oleg Romashin (:romaxa)]

That will bring a bit duplicate code, also I'm not sure will it work for other platforms, it might require interaction with internal nsWidget components... also it might break internal nsIwidget vkb open/close state

Comment 55

[Oleg Romashin (:romaxa)]

Also in order to create call from PluginParent to TabParent we are calling nsIMEContentManager which is calling PuppetWidget via nsIWidget interface.

if we are going to kill nsIWidget iface then we should create new PuppetWidget iface  or PluginParent -> TabParent  path in order to send request in right direction.

Comment 56

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can't you do your own IPC message from the X plugin code to the outer Qt widget? Not using PluginParent/TabParent etc, but using some other kind of communication channel?

Comment 57

[Oleg Romashin (:romaxa)]

> Can't you do your own IPC message from the X plugin code to the outer Qt
> widget? Not using PluginParent/TabParent etc, but using some other kind of
> communication channel?
I think that is not really possible... because plugin does not know even about content process... and in order to go through wall of processes we should use PluginChild->PluginParent and TabChild->TabParent
Also I think if there are some other way to do something directly from PluginChild->TabParent it could be security hole...

Comment 58

[Oleg Romashin (:romaxa)]

I don't think we have any other communication channel here... also in order to use X channel we should have window ID of parent process or do some other hacks...
Also I'm still not very sure why we can't use IME_STATUS for this case?
IME_STATUS_PLUGINS - when plugin focused in input field
IME_STATUS_DISABLED - when focused plugin does not have focus inside any editable field...

Comment 59

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Oleg Romashin (:romaxa) from comment #58)
> IME_STATUS_DISABLED - when focused plugin does not have focus inside any
> editable field...

because it's not "any", only "our".

Comment 60

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Oleg Romashin (:romaxa) from comment #58)
> I don't think we have any other communication channel here... also in order
> to use X channel we should have window ID of parent process or do some other
> hacks...

You can get the window ID of the parent window of the plugin, right? Why is that a hack?

I don't know much about low-level X window stuff but surely there's a way to send a custom message to an X window?

Comment 61

[Oleg Romashin (:romaxa)]

errr... it is windowless plugin with image rendering API, and we don't have any X-window involved....

Comment 62

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

Here is implementation of VKB activation using XProp notification handler...
roc can you review that or should we ask karl to check it?

Comment 63

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 556987 [details] [diff] [review]
Plugins VKB activation for Qt plugins

Review of attachment 556987 [details] [diff] [review]:
-----------------------------------------------------------------

I like this approach :-)

Comment 64

[Karl Tomlinson (:karlt)]

Are you wanting me to review the whole patch/approach or just the X11 IPC?
(I know little about IME so reviewing the whole approach may take some time.)

Comment 65

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

Found missing X11 ifdef...
Probably X11 part is enough, also would be nice to know about better naming for Xprops _NPAPI_PLUGIN_INPUT_FOCUSED - just something that came first in head

Comment 66

[Karl Tomlinson (:karlt)]

Comment on attachment 557356 [details] [diff] [review]
Plugins VKB activation for Qt plugins

> Probably X11 part is enough, also would be nice to know about better naming
> for Xprops _NPAPI_PLUGIN_INPUT_FOCUSED - just something that came first in
> head

Is the intention that the plugin will set this property directly?

_NPAPI_PLUGIN_INPUT_FOCUSED sounds fine to me.
Think about whether or not _NPAPI_PLUGIN_REQUEST_VKB may be better.

What happens if a plugin sets the property and then crashes?  AFAIK the
property would remain set, having an effect for other plugins/instances.

The other inter-client communication available with X11 is XSendEvent() with
an XClientMessageEvent.  That has the advantage of not requiring a round trip
to the server to fetch the value of the property.  The same advantage can
actually be obtained from XProperyEvent.state.  The difference is probably not
that important.  Using an XClientMessageEvent would require the filter to be
always installed.

Beware that using X11 for IPC means that there is no synchronization between
these messages and focus events, so out-of-order sequences need to be handled
correctly.

The default return value from GetPluginIMEState when there is no property is true.  Does that mean that the plugin needs to set the property to false before SetInputMode gets called, to ensure that the VKB is not opened?  How can it ensure that?

Does GetPluginIMEState need to be called from SetSoftwareKeyboardState at
times even when mIMEContext.mStatus != nsIWidget::IME_STATUS_PLUGIN.  This requires a round trip to the server, so it is best avoided if unnecessary.  I don't know how often SetInputMode is called.  I imagine it is only necessary to call GetPluginIMEState at the time x11EventFilter is installed (and in the event
handler).

>+    Atom actualType = 0;
>+    int actualFormat = 0;
>+    unsigned long nitems = 0;
>+    unsigned long bytes = 0;	

It shouldn't be necessary to initialize these.

>+        imeValue = (bool)data.asInt[0]

I would have thought explicit conversion would be unnecessary here. 
Is this to work around a narrowing-conversion compiler warning?

>+    //we are using fetchWIdCurrentAppWindow() instead of stored value (widCurrentAppWindow)
>+    //to eliminate a risk of race condition

I don't know where the fetchWIdCurrentAppWindow() is happening.
Is this comment still relevant?

>+    XEvent* event = reinterpret_cast<XEvent*>(message);

static_cast is fine here.

>+        XPropertyEvent* xevent = (XPropertyEvent*)event;

Usually, event.xproperty.

>+        static QCoreApplication::EventFilter previousEventFilter

Looks like x11EventFilter needs to chain up to previousEventFilter.

>+        if (xevent->atom == sPluginIMEAtom) {
>+            if (GetPluginIMEState(xevent->window)) {
>+                MozQWidget::requestVKB();
>+            } else {
>+                MozQWidget::hideVKB();
>+            }

Returning true here would make sense to me.

Comment 67

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

> Does GetPluginIMEState need to be called from SetSoftwareKeyboardState at
> times even when mIMEContext.mStatus != nsIWidget::IME_STATUS_PLUGIN.  This
mmm I do call GetPluginIMEState in SetSoftwareKeyboardState only when mIMEContext.mStatus == nsIWidget::IME_STATUS_PLUGIN..

Hopefully all comments addressed.

Comment 68

[Jeremias Bosch (:jbos)]

Comment on attachment 557418 [details] [diff] [review]
Plugins VKB activation for Qt plugins

Review of attachment 557418 [details] [diff] [review]:
-----------------------------------------------------------------

if (previousEventFilter)
    return previousEventFilter(message, result);

Looks good so far. you may want to add some { } here.

Comment 69

[Oleg Romashin (:romaxa)]

Ok, recently found that plugin can create fake QWidget and init QInputContext on plugin process side, but that approach  still have problem with VKB rotation... (known meegotouch issue)
So I guess we need current approach still until rotation issue solved  at least

Comment 70

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

Fixed comment, + added conversion to bool

Comment 71

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

oh, last change was wrong

Comment 72

[Oleg Romashin (:romaxa)]

Comment on attachment 557636 [details] [diff] [review]
Plugins VKB activation for Qt plugins

rrr, something wrong with this patch, found it crashy...

Comment 73

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

Ok, added check that prevFilter != x11Filter

Comment 74

[Oleg Romashin (:romaxa)]

Comment on attachment 557643 [details] [diff] [review]
Plugins VKB activation for Qt plugins

I guess I got initial review comments from karl, and fixed them, 
we need this patch soon, dougt could you double check it, so we can push it?

Comment 75

[Karl Tomlinson (:karlt)]

Comment on attachment 557643 [details] [diff] [review]
Plugins VKB activation for Qt plugins

Can you spell out the communication protocol please?

When does the plugin need to set the property?
When will it next need to set the property again with the same value
(i.e. after the browser has removed the property)?

Should the plugin ever unset the property?

With two plugins, what ensures that one plugin does not unset or change the
property that another plugin has set (and wants to remain set)?

>+        imeState = data.asBool[0] ? VKBOpen : VKBClose;

This needs to be data.asLong[0].

"If the returned format is 32, the returned data is represented as a long
array and should be cast to that type to obtain the elements."

>+        bool isOpen = aState == VKBOpen ? true : false;

Similarly this needs to be of type long.

>+x11EventFilter(void* message, long* result)
>+{
>+    if (sLastStatus != nsIWidget::IME_STATUS_PLUGIN) {
>+        return false;
>+    }

This test shouldn't be necessary as the filter is removed when
!IME_STATUS_PLUGIN, right?

>+    if (previousEventFilter && previousEventFilter != x11EventFilter) {
>+        return previousEventFilter(message, result);

It is disturbing that previousEventFilter can become x11EventFilter.
Sounds like a better mechanism is needed to determine whether a filter is
already installed.

>+    if (mIMEContext.mStatus == nsIWidget::IME_STATUS_PLUGIN && !previousEventFilter) {
>+        // Install event filter for listening Plugin IME state changes
>+        previousEventFilter = QCoreApplication::instance()->setEventFilter(x11EventFilter);

I assume !previousEventFilter is not a very useful test because there may not
have been a previous event filter before x11EventFilter was installed.

Comment 76

[Karl Tomlinson (:karlt)]

(In reply to Oleg Romashin (:romaxa) from comment #69)
> Ok, recently found that plugin can create fake QWidget and init
> QInputContext on plugin process side, but that approach  still have problem
> with VKB rotation... (known meegotouch issue)

How does the approach in patches here avoid the rotation problem?
What is different that the plugin cannot do?

Comment 77

[Oleg Romashin (:romaxa)]

> How does the approach in patches here avoid the rotation problem?
> What is different that the plugin cannot do?

With current patches we are using IME context created in UI process, where Meegotouch window/widget created, focused and rotated...
I tried to create MWindow on plugin process, setPortrait mode to that widget, but that did not work :(, as soon that will be possible we will kill all this functionality.

Comment 78

[Oleg Romashin (:romaxa)]

Tested it also on simple example and keyboard is not rotated if MSceneWindow not connected to raised QGraphicsView... it is possible to make it works by calling show and hide to QGraphicsView, after that it starts working... (that seems initializing some X-Window backed stuff and does communication with that window)
I don't think it is good if plugin will create window then show it and hide... user will notice that.

Comment 79

[Oleg Romashin (:romaxa)]

> Can you spell out the communication protocol please?
> 
> When does the plugin need to set the property?
When plugin focused by user and receive mouse release, or focus events without mouse events (dom focus for example).
depends on release point it should set property according to plugin internal IME state (text field or not).

> When will it next need to set the property again with the same value
if plugin has been unfocused and got new focus.

> Should the plugin ever unset the property?

No, plugin should just set property value according to mouse release target node (editable state).

> With two plugins, what ensures that one plugin does not unset or change the
> property that another plugin has set (and wants to remain set)?

When we switch between plugin windows, they will get focus/unfocus events, also we cannot have 2 focused plugin at the same time (IIRC). so current property set will be related to current focused plugin

Comment 80

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

Fixed review comments

Comment 81

[Karl Tomlinson (:karlt)]

(In reply to Oleg Romashin (:romaxa) from comment #79)
> > With two plugins, what ensures that one plugin does not unset or change the
> > property that another plugin has set (and wants to remain set)?
> 
> When we switch between plugin windows, they will get focus/unfocus events,
> also we cannot have 2 focused plugin at the same time (IIRC). so current
> property set will be related to current focused plugin

Yes, only one plugin can have focus at one time.

The plugin will need to call XSync before returning from the event handler, if it has set or changed the property.  (Otherwise, its property change request may be effected after another plugin has received focus and performed its request.)

Comment 82

[Oleg Romashin (:romaxa)]

> The plugin will need to call XSync before returning from the event handler,
> if it has set or changed the property.  (Otherwise, its property change
Ok, make sense, will add it into plugin.

Comment 83

[Karl Tomlinson (:karlt)]

Comment on attachment 558582 [details] [diff] [review]
Plugins VKB activation for Qt plugins

I haven't reviewed the mozqwidget changes.
Please ensure that they either have been or will be reviewed by someone else.

>+static PRUint32 sLastStatus = nsIWidget::IME_STATUS_DISABLED;

>+    sLastStatus = aContext.mStatus;

Set but not used.

>-    NS_ENSURE_TRUE(mWidget, NS_ERROR_FAILURE);
>-

Moving this to SetSoftwareKeyboardState has been done in such a way as to
change the return code of SetInputMode.
Please get someone who knows this code to review this change.

>+    // SetSoftwareKeyboardState using mIMEContext,
>+    // therefore before calling that we should remember
>+    // aContext into mIMEContext

"SetSoftwareKeyboardState uses mIMEContext,
so, before calling that, record aContext in mIMEContext."

>+        SetVKBState(GetViewWidget()->winId(), VKBUndefined);

An XSync is necessary here to ensure this is processed before a plugin receives focus.

>+            SetSoftwareKeyboardState(true);
>             break;
>         default:
>-            mWidget->hideVKB();
>-            break;
>+            SetSoftwareKeyboardState(false);
>+        break;

true/false are of type bool, but SetSoftwareKeyboardState expects PRBool.
Make these of consistent type.

Leave the indentation of "break" as before.

(In reply to Karl Tomlinson (:karlt) from comment #66)
> The default return value from GetPluginIMEState when there is no property is
> true.  Does that mean that the plugin needs to set the property to false
> before SetInputMode gets called, to ensure that the VKB is not opened?  How
> can it ensure that?

I don't think you answered this question.

Is it just the delay that stops the VKB opening, or is SetInputMode() called
strictly after the plugin receives focus?

Comment 84

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins

> I haven't reviewed the mozqwidget changes.
> Please ensure that they either have been or will be reviewed by someone else.
It was checked by original author already.

> >-    NS_ENSURE_TRUE(mWidget, NS_ERROR_FAILURE);
> >-
> change the return code of SetInputMode.

Actually we have done it for previous version of patch, where we moved direct mWidget use from SetInputMode to SetSfkbState and that was public, now it is private and SetInputMode, also we don't have direct mWidget use anymore, so we will update our mContext, do whatever we can do, and not crash even if mWidget is null
Also checked SetInputMode callers and there are no return value check.


> Is it just the delay that stops the VKB opening, or is SetInputMode() called
> strictly after the plugin receives focus?

SetInputMode is strictly called when plugin receives focus with IME_STATUS_PLUGIN, and when plugin loose focus, that is starting timeout and plugin should update plugin-VKB substate, if plugin did not update IME substate via X-Prop, then VKB will be opened for plugin (after timeout expired).
http://mxr.mozilla.org/mozilla-central/source/widget/public/nsIWidget.h#1302

Comment 85

[Karl Tomlinson (:karlt)]

(In reply to Oleg Romashin (:romaxa) from comment #84)
> > Is it just the delay that stops the VKB opening, or is SetInputMode() called
> > strictly after the plugin receives focus?
> 
> SetInputMode is strictly called when plugin receives focus with
> IME_STATUS_PLUGIN,

Is SetInputMode called before or after the plugin receives focus?

Comment 86

[Karl Tomlinson (:karlt)]

Comment on attachment 558724 [details] [diff] [review]
Plugins VKB activation for Qt plugins

>Bug 590299 - Virtual keyboard is not invoked for input fields in Qt plugins r=karlt

Some other reviewer needs to be mentioned here for the mozqwidget changes.

>+    // "SetSoftwareKeyboardState uses mIMEContext,
>+    // so, before calling that, record aContext in mIMEContext."

Remove the quotes here.

>+nsresult
>+nsWindow::SetSoftwareKeyboardState(PRBool aOpen)

Remove the unused return value and make this a void method, please.

r+, but I want the answer to comment 85.

If SetInputMode is called before the plugin receives focus, then please add a
comment before the "openDelay" code saying that this gives the plugin a chance
to prevent the VKB from opening.

Comment 87

[Oleg Romashin (:romaxa)]

Actually there are 2 modes, one is in process another is IPC, and in IPC mode we cannot predict what will happen first... when content process detect focus state, it send one IPC msg to plugin child process and another IPC msg to UI process...

Comment 88

[Oleg Romashin (:romaxa)]

In IPC mode I have SetInputMode first and Plugin focus/mouse press later

Comment 89

[Oleg Romashin (:romaxa)]

Attachment: Fixed Last nits, to PUSH

Comment 90

[Oleg Romashin (:romaxa)]

Attachment: Plugins VKB activation for Qt plugins, Fixed Last nits, to PUSH

Ups, wrong version, removed return values from void SetSoftwareKeyboardState.

Comment 91

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/789165341d81

Comment 92

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/789165341d81