From 76e86981d8181cbcca9898047fbdc1d17cbbc9cd Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Fri, 20 Mar 2015 01:52:24 +0900 Subject: [PATCH] Bug 1143197 part.1 Use current IM context at handling key events rather than active IM context r=m_kato --- widget/gtk/nsGtkIMModule.cpp | 79 ++++++++++++++++++++++++------------ widget/gtk/nsGtkIMModule.h | 12 +++++- 2 files changed, 64 insertions(+), 27 deletions(-) diff --git a/widget/gtk/nsGtkIMModule.cpp b/widget/gtk/nsGtkIMModule.cpp index 1c2a8a8499be..194acd1c2034 100644 --- a/widget/gtk/nsGtkIMModule.cpp +++ b/widget/gtk/nsGtkIMModule.cpp @@ -21,6 +21,12 @@ using namespace mozilla::widget; #ifdef PR_LOGGING PRLogModuleInfo* gGtkIMLog = nullptr; +static const char* +GetBoolName(bool aBool) +{ + return aBool ? "true" : "false"; +} + static const char* GetRangeTypeName(uint32_t aRangeType) { @@ -56,6 +62,19 @@ GetEnabledStateName(uint32_t aState) return "UNKNOWN ENABLED STATUS!!"; } } + +static const char* +GetEventType(GdkEventKey* aKeyEvent) +{ + switch (aKeyEvent->type) { + case GDK_KEY_PRESS: + return "GDK_KEY_PRESS"; + case GDK_KEY_RELEASE: + return "GDK_KEY_RELEASE"; + default: + return "Unknown"; + } +} #endif const static bool kUseSimpleContextDefault = MOZ_WIDGET_GTK == 2; @@ -298,8 +317,9 @@ nsGtkIMModule::OnBlurWindow(nsWindow* aWindow) } PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, - ("GtkIMModule(%p): OnBlurWindow, aWindow=%p, mLastFocusedWindow=%p, mIsIMFocused=%s", - this, aWindow, mLastFocusedWindow, mIsIMFocused ? "YES" : "NO")); + ("GtkIMModule(%p): OnBlurWindow, aWindow=%p, mLastFocusedWindow=%p, " + "mIsIMFocused=%s", + this, aWindow, mLastFocusedWindow, GetBoolName(mIsIMFocused))); if (!mIsIMFocused || mLastFocusedWindow != aWindow) { return; @@ -320,13 +340,12 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent, } PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, - ("GtkIMModule(%p): OnKeyEvent, aCaller=%p, aKeyDownEventWasSent=%s", - this, aCaller, aKeyDownEventWasSent ? "TRUE" : "FALSE")); - PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, - (" aEvent: type=%s, keyval=%s, unicode=0x%X", - aEvent->type == GDK_KEY_PRESS ? "GDK_KEY_PRESS" : - aEvent->type == GDK_KEY_RELEASE ? "GDK_KEY_RELEASE" : "Unknown", - gdk_keyval_name(aEvent->keyval), + ("GtkIMModule(%p): OnKeyEvent, aCaller=%p, aKeyDownEventWasSent=%s, " + "mCompositionState=%s, current context=%p, active context=%p, " + "aEvent(%p): { type=%s, keyval=%s, unicode=0x%X }", + this, aCaller, GetBoolName(aKeyDownEventWasSent), + GetCompositionStateName(), GetCurrentContext(), GetActiveContext(), + aEvent, GetEventType(aEvent), gdk_keyval_name(aEvent->keyval), gdk_keyval_to_unicode(aEvent->keyval))); if (aCaller != mLastFocusedWindow) { @@ -336,8 +355,10 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent, return false; } - GtkIMContext* activeContext = GetActiveContext(); - if (MOZ_UNLIKELY(!activeContext)) { + // Even if old IM context has composition, key event should be sent to + // current context since the user expects so. + GtkIMContext* currentContext = GetCurrentContext(); + if (MOZ_UNLIKELY(!currentContext)) { PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, (" FAILED, there are no context")); return false; @@ -347,7 +368,7 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent, mFilterKeyEvent = true; mProcessingKeyEvent = aEvent; gboolean isFiltered = - gtk_im_context_filter_keypress(activeContext, aEvent); + gtk_im_context_filter_keypress(currentContext, aEvent); mProcessingKeyEvent = nullptr; // We filter the key event if the event was not committed (because @@ -357,7 +378,7 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent, // composed characters. bool filterThisEvent = isFiltered && mFilterKeyEvent; - if (IsComposing() && !isFiltered) { + if (IsComposingOnCurrentContext() && !isFiltered) { if (aEvent->type == GDK_KEY_PRESS) { if (!mDispatchedCompositionString.IsEmpty()) { // If there is composition string, we shouldn't dispatch @@ -371,7 +392,7 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent, // IM. For compromising this issue, we should dispatch // compositionend event, however, we don't need to reset IM // actually. - DispatchCompositionCommitEvent(activeContext, &EmptyString()); + DispatchCompositionCommitEvent(currentContext, &EmptyString()); filterThisEvent = false; } } else { @@ -382,9 +403,10 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent, } PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, - (" filterThisEvent=%s (isFiltered=%s, mFilterKeyEvent=%s)", - filterThisEvent ? "TRUE" : "FALSE", isFiltered ? "YES" : "NO", - mFilterKeyEvent ? "YES" : "NO")); + (" filterThisEvent=%s (isFiltered=%s, mFilterKeyEvent=%s), " + "mCompositionState=%s", + GetBoolName(filterThisEvent), GetBoolName(isFiltered), + GetBoolName(mFilterKeyEvent), GetCompositionStateName())); return filterThisEvent; } @@ -395,8 +417,8 @@ nsGtkIMModule::OnFocusChangeInGecko(bool aFocus) PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, ("GtkIMModule(%p): OnFocusChangeInGecko, aFocus=%s, " "mCompositionState=%s, mIsIMFocused=%s", - this, aFocus ? "YES" : "NO", GetCompositionStateName(), - mIsIMFocused ? "YES" : "NO")); + this, GetBoolName(aFocus), GetCompositionStateName(), + GetBoolName(mIsIMFocused))); // We shouldn't carry over the removed string to another editor. mSelectedString.Truncate(); @@ -407,7 +429,7 @@ nsGtkIMModule::ResetIME() { PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, ("GtkIMModule(%p): ResetIME, mCompositionState=%s, mIsIMFocused=%s", - this, GetCompositionStateName(), mIsIMFocused ? "YES" : "NO")); + this, GetCompositionStateName(), GetBoolName(mIsIMFocused))); GtkIMContext* activeContext = GetActiveContext(); if (MOZ_UNLIKELY(!activeContext)) { @@ -646,7 +668,7 @@ nsGtkIMModule::Blur() { PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, ("GtkIMModule(%p): Blur, mIsIMFocused=%s", - this, mIsIMFocused ? "YES" : "NO")); + this, GetBoolName(mIsIMFocused))); if (!mIsIMFocused) { return; @@ -889,11 +911,13 @@ nsGtkIMModule::OnCommitCompositionNative(GtkIMContext *aContext, PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, ("GtkIMModule(%p): OnCommitCompositionNative, aContext=%p, " - "current context=%p, commitString=\"%s\"", - this, aContext, GetCurrentContext(), commitString)); + "current context=%p, active context=%p, commitString=\"%s\", " + "mProcessingKeyEvent=%p, IsComposingOn(aContext)=%s", + this, aContext, GetCurrentContext(), GetActiveContext(), commitString, + mProcessingKeyEvent, GetBoolName(IsComposingOn(aContext)))); // See bug 472635, we should do nothing if IM context doesn't match. - if (GetCurrentContext() != aContext) { + if (!IsValidContext(aContext)) { PR_LOG(gGtkIMLog, PR_LOG_ALWAYS, (" FAILED, given context doesn't match")); return; @@ -904,14 +928,17 @@ nsGtkIMModule::OnCommitCompositionNative(GtkIMContext *aContext, // signal, we would dispatch compositionstart, text, compositionend // events with empty string. Of course, they are unnecessary events // for Web applications and our editor. - if (!IsComposing() && !commitString[0]) { + if (!IsComposingOn(aContext) && !commitString[0]) { return; } // If IME doesn't change their keyevent that generated this commit, // don't send it through XIM - just send it as a normal key press // event. - if (!IsComposing() && mProcessingKeyEvent) { + // NOTE: While a key event is being handled, this might be caused on + // current context. Otherwise, this may be caused on active context. + if (!IsComposingOn(aContext) && mProcessingKeyEvent && + aContext == GetCurrentContext()) { char keyval_utf8[8]; /* should have at least 6 bytes of space */ gint keyval_utf8_len; guint32 keyval_unicode; diff --git a/widget/gtk/nsGtkIMModule.h b/widget/gtk/nsGtkIMModule.h index 2e4b7c01d444..f5f5deeeba5a 100644 --- a/widget/gtk/nsGtkIMModule.h +++ b/widget/gtk/nsGtkIMModule.h @@ -132,11 +132,21 @@ protected: }; eCompositionState mCompositionState; - bool IsComposing() + bool IsComposing() const { return (mCompositionState != eCompositionState_NotComposing); } + bool IsComposingOn(GtkIMContext* aContext) const + { + return IsComposing() && mComposingContext == aContext; + } + + bool IsComposingOnCurrentContext() const + { + return IsComposingOn(GetCurrentContext()); + } + bool EditorHasCompositionString() { return (mCompositionState ==