Bug 1525867 - Make IMContextWrapper::SetTextRange() not ignore composition clause even if no visual styles are specified r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 18 Mar 2019 03:00:23 +0000
changeset 525619 075d0e6434cc74054eeefb2a768d7629e9f2de95
parent 525618 091fdeb8c9ca58a3ab6c00368828448fed62b4bd
child 525620 58a76dfcf9d8014cc383b0399db2d1f1a21bace7
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1525867
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1525867 - Make IMContextWrapper::SetTextRange() not ignore composition clause even if no visual styles are specified r=m_kato We've ignored clauses whose visual styles are not specified. However, kinput2 with XIM protocol does not specify any styles to non-selected clauses. Therefore, we fail to dispatch eCompositionChange events if there is 2 or more clauses. Note that the log in the bug indicates that we may set selected clause type to`TextRangeType::eConvertedClause` and last clause type to `TextRangeType::eSelectedClause` because caret is always put at end of composition string. However, this should not problem for now because nobody except plugins on Windows refer this information. Differential Revision: https://phabricator.services.mozilla.com/D23464
widget/gtk/IMContextWrapper.cpp
--- a/widget/gtk/IMContextWrapper.cpp
+++ b/widget/gtk/IMContextWrapper.cpp
@@ -2460,44 +2460,66 @@ already_AddRefed<TextRangeArray> IMConte
              "be allocated",
              this));
     pango_attr_list_unref(feedback_list);
     g_free(preedit_string);
     return textRangeArray.forget();
   }
 
   uint32_t minOffsetOfClauses = aCompositionString.Length();
+  uint32_t maxOffsetOfClauses = 0;
   do {
     TextRange range;
     if (!SetTextRange(iter, preedit_string, caretOffsetInUTF16, range)) {
       continue;
     }
     MOZ_ASSERT(range.Length());
     minOffsetOfClauses = std::min(minOffsetOfClauses, range.mStartOffset);
+    maxOffsetOfClauses = std::max(maxOffsetOfClauses, range.mEndOffset);
     textRangeArray->AppendElement(range);
   } while (pango_attr_iterator_next(iter));
 
   // If the IME doesn't define clause from the start of the composition,
   // we should insert dummy clause information since TextRangeArray assumes
   // that there must be a clause whose start is 0 when there is one or
   // more clauses.
   if (minOffsetOfClauses) {
     TextRange dummyClause;
     dummyClause.mStartOffset = 0;
     dummyClause.mEndOffset = minOffsetOfClauses;
     dummyClause.mRangeType = TextRangeType::eRawClause;
     textRangeArray->InsertElementAt(0, dummyClause);
+    maxOffsetOfClauses = std::max(maxOffsetOfClauses, dummyClause.mEndOffset);
     MOZ_LOG(gGtkIMLog, LogLevel::Warning,
             ("0x%p   CreateTextRangeArray(), inserting a dummy clause "
              "at the beginning of the composition string mStartOffset=%u, "
              "mEndOffset=%u, mRangeType=%s",
              this, dummyClause.mStartOffset, dummyClause.mEndOffset,
              ToChar(dummyClause.mRangeType)));
   }
 
+  // If the IME doesn't define clause at end of the composition, we should
+  // insert dummy clause information since TextRangeArray assumes that there
+  // must be a clase whose end is the length of the composition string when
+  // there is one or more clauses.
+  if (!textRangeArray->IsEmpty() &&
+      maxOffsetOfClauses < aCompositionString.Length()) {
+    TextRange dummyClause;
+    dummyClause.mStartOffset = maxOffsetOfClauses;
+    dummyClause.mEndOffset = aCompositionString.Length();
+    dummyClause.mRangeType = TextRangeType::eRawClause;
+    textRangeArray->AppendElement(dummyClause);
+    MOZ_LOG(gGtkIMLog, LogLevel::Warning,
+            ("0x%p   CreateTextRangeArray(), inserting a dummy clause "
+             "at the end of the composition string mStartOffset=%u, "
+             "mEndOffset=%u, mRangeType=%s",
+             this, dummyClause.mStartOffset, dummyClause.mEndOffset,
+             ToChar(dummyClause.mRangeType)));
+  }
+
   TextRange range;
   range.mStartOffset = range.mEndOffset = caretOffsetInUTF16;
   range.mRangeType = TextRangeType::eCaret;
   textRangeArray->AppendElement(range);
   MOZ_LOG(
       gGtkIMLog, LogLevel::Debug,
       ("0x%p   CreateTextRangeArray(), mStartOffset=%u, "
        "mEndOffset=%u, mRangeType=%s",
@@ -2657,24 +2679,16 @@ bool IMContextWrapper::SetTextRange(Pang
    *
    * However, this rules are odd since there can be two or more selected
    * clauses.  Additionally, our old rules caused that IME developers/users
    * cannot specify composition string style as they want.
    *
    * So, we shouldn't guess the meaning from its visual style.
    */
 
-  if (!attrUnderline && !attrForeground && !attrBackground) {
-    MOZ_LOG(gGtkIMLog, LogLevel::Warning,
-            ("0x%p   SetTextRange(), FAILED, due to no attr, "
-             "aTextRange= { mStartOffset=%u, mEndOffset=%u }",
-             this, aTextRange.mStartOffset, aTextRange.mEndOffset));
-    return false;
-  }
-
   // If the range covers whole of composition string and the caret is at
   // the end of the composition string, the range is probably not converted.
   if (!utf8ClauseStart &&
       utf8ClauseEnd == static_cast<gint>(strlen(aUTF8CompositionString)) &&
       aTextRange.mEndOffset == aUTF16CaretOffset) {
     aTextRange.mRangeType = TextRangeType::eRawClause;
   }
   // Typically, the caret is set at the start of the selected clause.