Bug 1312649 part.2 IMEInputHandler::GetVaildAttributesForMarkedText() should return non-empty array r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 07 Nov 2016 16:19:41 +0900
changeset 434740 d7ca7e8abe170613c51f9cb62b19d7598e459736
parent 434739 a6834be4aeabffb84d9fc6918b04d90fe2d71cdb
child 536098 0ad2460816c395f1a5241dfcf95afa687be88d5e
push id34806
push usermasayuki@d-toybox.com
push dateMon, 07 Nov 2016 08:58:54 +0000
reviewersm_kato
bugs1312649
milestone52.0a1
Bug 1312649 part.2 IMEInputHandler::GetVaildAttributesForMarkedText() should return non-empty array r?m_kato Vietnamese Telex perhaps referes this result and change its behavior. When typying something, Telex starts composition on Chrome but may not behave so on Gecko. Fortunately, Chromium just returns some attributes when validAttributesForMarkedText: of NSTextInputClient [1] but it doesn't return these styles when attributedSubstringForProposedRange: of NSTextInputClient is called (always returns non-styled plain text) [2]. Therefore, this patch does not touch IMEInputHandler::GetAttributedSubstringFromRange(). *1 <https://chromium.googlesource.com/chromium/src/+/7d85f23cb0235db06b0b6c2de1dc29ae5eaeb8f5/content/browser/renderer_host/render_widget_host_view_mac.mm#2936> *2 <https://chromium.googlesource.com/chromium/src/+/7d85f23cb0235db06b0b6c2de1dc29ae5eaeb8f5/content/browser/renderer_host/render_widget_host_view_mac.mm#3036> MozReview-Commit-ID: 1gPIiu4Qbud
widget/cocoa/TextInputHandler.mm
--- a/widget/cocoa/TextInputHandler.mm
+++ b/widget/cocoa/TextInputHandler.mm
@@ -3267,16 +3267,20 @@ IMEInputHandler::GetAttributedSubstringF
   }
 
   RefPtr<IMEInputHandler> kungFuDeathGrip(this);
 
   // If we're in composing, the queried range may be in the composition string.
   // In such case, we should use mIMECompositionString since if the composition
   // string is handled by a remote process, the content cache may be out of
   // date.
+  // XXX Should we set composition string attributes?  Although, Blink claims
+  //     that some attributes of marked text are supported, but they return
+  //     just marked string without any style.  So, let's keep current behavior
+  //     at least for now.
   NSUInteger compositionLength =
     mIMECompositionString ? [mIMECompositionString length] : 0;
   if (mIMECompositionStart != UINT32_MAX &&
       mIMECompositionStart >= aRange.location &&
       mIMECompositionStart + compositionLength <=
         aRange.location + aRange.length) {
     NSRange range =
       NSMakeRange(aRange.location - mIMECompositionStart, aRange.length);
@@ -3588,32 +3592,38 @@ IMEInputHandler::CharacterIndexForPoint(
     return NSNotFound;
   }
 
   return charAt.mReply.mOffset;
 
   NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(NSNotFound);
 }
 
+extern "C" {
+extern NSString *NSTextInputReplacementRangeAttributeName;
+}
+
 NSArray*
 IMEInputHandler::GetValidAttributesForMarkedText()
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NIL;
 
   MOZ_LOG(gLog, LogLevel::Info,
     ("%p IMEInputHandler::GetValidAttributesForMarkedText", this));
 
-  //RefPtr<IMEInputHandler> kungFuDeathGrip(this);
-
-  //return [NSArray arrayWithObjects:NSUnderlineStyleAttributeName,
-  //                                 NSMarkedClauseSegmentAttributeName,
-  //                                 NSTextInputReplacementRangeAttributeName,
-  //                                 nil];
-  // empty array; we don't support any attributes right now
-  return [NSArray array];
+  // Return same attributes as Chromium (see render_widget_host_view_mac.mm)
+  // because most IMEs must be tested with Safari (OS default) and Chrome
+  // (having most market share).  Therefore, we need to follow their behavior.
+  // XXX It might be better to reuse an array instance for this result because
+  //     this may be called a lot.  Note that Chromium does so.
+  return [NSArray arrayWithObjects:NSUnderlineStyleAttributeName,
+                                   NSUnderlineColorAttributeName,
+                                   NSMarkedClauseSegmentAttributeName,
+                                   NSTextInputReplacementRangeAttributeName,
+                                   nil];
 
   NS_OBJC_END_TRY_ABORT_BLOCK_NIL;
 }
 
 
 #pragma mark -