Bug 1453629 - nsIWidget::GetNativeIMEContext() should return pseudo IME context if TextInputProcessor has a composition on the widget r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 17 Jul 2018 22:31:51 +0900
changeset 819597 c68fa3c35f9724b908befd2667d42164ce0a5c3f
parent 819596 b4dc5b34fe6ff80861abb665455c2d72dfb00eb8
child 819734 40cb6e6e1f7a72437921f05a372fb338c9975fa9
push id116591
push usermasayuki@d-toybox.com
push dateWed, 18 Jul 2018 07:01:58 +0000
reviewersm_kato
bugs1453629
milestone63.0a1
Bug 1453629 - nsIWidget::GetNativeIMEContext() should return pseudo IME context if TextInputProcessor has a composition on the widget r?m_kato Key of TextCompositionArrary to search composition on widget is native IME context. However, if TextInputProcessor dispatches composition events via TextEventDispatcher, TextEventDispatcher::InitEvent() sets native IME context of dispatching composition events to pseudo IME context (that's raw pointer of TextEventDispatcher and process ID). IMEStateManager::DispatchCompositionEvent() looks for existing TextComposition with native IME context stored in WidgetCompositionEvent before dispatching an event. However, after dispatching it, it looks for remaining TextComposition instance with widget stored in WidgetCompositionEvent. Then, TextCompositionArrary::IndexOf(nsIWidget*) will look for an instance with the result of nsIWidget::GetNativeIMEContext() and this never returns actual native IME context. Therefore, IMEStateManager::DispatchCompositionEvent() always fails to remove TextComposition instance from the array even after a test composition is finished. This patch moves nsIWidget::GetNativeIMEContext() to nsBaseWidget to refer nsBaseWidget::mTextEventDispatcher makes it return pseudo IME context if TextInputProcessor has input transaction. Therefore, IMEStateManager becomes always removes TextComposition from the array when every composition ends. MozReview-Commit-ID: H1PCtPjBYJR
dom/base/test/chrome/window_nsITextInputProcessor.xul
widget/nsBaseWidget.cpp
widget/nsBaseWidget.h
widget/nsIWidget.h
--- a/dom/base/test/chrome/window_nsITextInputProcessor.xul
+++ b/dom/base/test/chrome/window_nsITextInputProcessor.xul
@@ -152,17 +152,19 @@ function runBeginInputTransactionMethodT
   ok(TIP2.beginInputTransaction(otherWindow, simpleCallback),
      description + "TIP2.beginInputTransaction(otherWindow) should succeed because there is composition synthesized by TIP1 but it's in other window");
   ok(TIP2.beginInputTransactionForTests(otherWindow),
      description + "TIP2.beginInputTransactionForTests(otherWindow) should succeed because there is composition synthesized by TIP1 but it's in other window");
 
   // Let's confirm that the composing string is NOT committed by above tests.
   TIP1.commitComposition();
   is(input.value, composingStr,
-     description + "TIP1.commitString() without specifying commit string should be committed with the last composing string");
+     description + "TIP1.commitString() without specifying commit string should commit current composition with the last composing string");
+  ok(!TIP1.hasComposition,
+     description + "TIP1.commitString() without specifying commit string should've end composition");
 
   ok(TIP1.beginInputTransaction(window, simpleCallback),
      description + "TIP1.beginInputTransaction() should succeed because there is no composition #2");
   ok(TIP1.beginInputTransactionForTests(window),
      description + "TIP1.beginInputTransactionForTests() should succeed because there is no composition #2");
   ok(TIP2.beginInputTransactionForTests(window),
      description + "TIP2.beginInputTransactionForTests() should succeed because the composition was already committed #2");
 
--- a/widget/nsBaseWidget.cpp
+++ b/widget/nsBaseWidget.cpp
@@ -1928,16 +1928,32 @@ void
 nsBaseWidget::EnsureTextEventDispatcher()
 {
   if (mTextEventDispatcher) {
     return;
   }
   mTextEventDispatcher = new TextEventDispatcher(this);
 }
 
+nsIWidget::NativeIMEContext
+nsBaseWidget::GetNativeIMEContext()
+{
+  if (mTextEventDispatcher && mTextEventDispatcher->GetPseudoIMEContext()) {
+    // If we already have a TextEventDispatcher and it's working with
+    // a TextInputProcessor, we need to return pseudo IME context since
+    // TextCompositionArray::IndexOf(nsIWidget*) should return a composition
+    // on the pseudo IME context in such case.
+    NativeIMEContext pseudoIMEContext;
+    pseudoIMEContext.InitWithRawNativeIMEContext(
+                       mTextEventDispatcher->GetPseudoIMEContext());
+    return pseudoIMEContext;
+  }
+  return NativeIMEContext(this);
+}
+
 nsIWidget::TextEventDispatcher*
 nsBaseWidget::GetTextEventDispatcher()
 {
   EnsureTextEventDispatcher();
   return mTextEventDispatcher;
 }
 
 void*
@@ -2436,22 +2452,16 @@ nsBaseWidget::DefaultFillScrollCapture(D
   aSnapshotDrawTarget->FillRect(
     gfx::Rect(0, 0, dtSize.width, dtSize.height),
     gfx::ColorPattern(gfx::Color::FromABGR(kScrollCaptureFillColor)),
     gfx::DrawOptions(1.f, gfx::CompositionOp::OP_SOURCE));
   aSnapshotDrawTarget->Flush();
 }
 #endif
 
-nsIWidget::NativeIMEContext
-nsIWidget::GetNativeIMEContext()
-{
-  return NativeIMEContext(this);
-}
-
 const IMENotificationRequests&
 nsIWidget::IMENotificationRequestsRef()
 {
   TextEventDispatcher* dispatcher = GetTextEventDispatcher();
   return dispatcher->IMENotificationRequestsRef();
 }
 
 nsresult
--- a/widget/nsBaseWidget.h
+++ b/widget/nsBaseWidget.h
@@ -293,16 +293,17 @@ public:
   CreateChild(const LayoutDeviceIntRect& aRect,
               nsWidgetInitData* aInitData = nullptr,
               bool aForceUseIWidgetParent = false) override;
   virtual void            AttachViewToTopLevel(bool aUseAttachedEvents) override;
   virtual nsIWidgetListener* GetAttachedWidgetListener() override;
   virtual void               SetAttachedWidgetListener(nsIWidgetListener* aListener) override;
   virtual nsIWidgetListener* GetPreviouslyAttachedWidgetListener() override;
   virtual void               SetPreviouslyAttachedWidgetListener(nsIWidgetListener* aListener) override;
+  virtual NativeIMEContext GetNativeIMEContext() override;
   TextEventDispatcher* GetTextEventDispatcher() final;
   virtual TextEventDispatcherListener*
     GetNativeTextEventDispatcherListener() override;
   virtual void ZoomToRect(const uint32_t& aPresShellId,
                           const FrameMetrics::ViewID& aViewId,
                           const CSSRect& aRect,
                           const uint32_t& aFlags) override;
   // Dispatch an event that must be first be routed through APZ.
--- a/widget/nsIWidget.h
+++ b/widget/nsIWidget.h
@@ -1846,17 +1846,17 @@ public:
      */
     virtual InputContext GetInputContext() = 0;
 
     /**
      * Get native IME context.  This is different from GetNativeData() with
      * NS_RAW_NATIVE_IME_CONTEXT, the result is unique even if in a remote
      * process.
      */
-    virtual NativeIMEContext GetNativeIMEContext();
+    virtual NativeIMEContext GetNativeIMEContext() = 0;
 
     /*
      * Given a WidgetKeyboardEvent, this method synthesizes a corresponding
      * native (OS-level) event for it. This method allows tests to simulate
      * keystrokes that trigger native key bindings (which require a native
      * event).
      */
     virtual MOZ_MUST_USE nsresult