MacViews: delete entire composed glyph when deleting backward

This matches NSTextView's behavior by creating a CFString and asking
what Cocoa thinks should happen. This also handles UTF-16 surrogate
pairs, so this change splits that check to the non-Mac code path.

Bug: 826794
Change-Id: I8fce5510e3cd483533699edd1ba33118ce55b44e
Reviewed-on: https://chromium-review.googlesource.com/986475
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548769}

ui/views/controls/textfield/textfield_model.cc

@@ -16,6 +16,7 @@
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/gfx/range/range.h"
 #include "ui/gfx/utf16_indexing.h"
+#include "ui/views/style/platform_style.h"
 
 namespace views {
 
@@ -406,10 +407,8 @@ bool TextfieldModel::Backspace(bool add_to_kill_buffer) {
   }
   size_t cursor_position = GetCursorPosition();
   if (cursor_position > 0) {
-    // Delete one code point, which may be two UTF-16 words.
-    size_t previous_grapheme_index =
-        gfx::UTF16OffsetToIndex(text(), cursor_position, -1);
-    gfx::Range range_to_delete(cursor_position, previous_grapheme_index);
+    gfx::Range range_to_delete(
+        PlatformStyle::RangeToDeleteBackwards(text(), cursor_position));
     if (add_to_kill_buffer)
       SetKillBuffer(GetTextFromRange(range_to_delete));
     ExecuteAndRecordDelete(range_to_delete, true);

ui/views/controls/textfield/textfield_model_unittest.cc

@@ -233,6 +233,38 @@ TEST_F(TextfieldModelTest, EditString_ComplexScript) {
   EXPECT_TRUE(model.Backspace());
   EXPECT_EQ(base::WideToUTF16(L"\x002C\x0020\x05D1\x05BC\x05B7\x05E9"),
             model.text());
+
+  // Halfwidth katakana ﾀﾞ:
+  // "HALFWIDTH KATAKANA LETTER TA" + "HALFWIDTH KATAKANA VOICED SOUND MARK"
+  // ("ABC" prefix as sanity check that the entire string isn't deleted).
+  model.SetText(base::WideToUTF16(L"ABC\xFF80\xFF9E"));
+  MoveCursorTo(model, model.text().length());
+  model.Backspace();
+#if defined(OS_MACOSX)
+  // On Mac, the entire cluster should be deleted to match
+  // NSTextField behavior.
+  EXPECT_EQ(base::WideToUTF16(L"ABC"), model.text());
+  EXPECT_EQ(3U, model.GetCursorPosition());
+#else
+  EXPECT_EQ(base::WideToUTF16(L"ABC\xFF80"), model.text());
+  EXPECT_EQ(4U, model.GetCursorPosition());
+#endif
+
+  // Emoji with Fitzpatrick modifier:
+  // 'BOY' + 'EMOJI MODIFIER FITZPATRICK TYPE-5'
+  model.SetText(base::WideToUTF16(L"\U0001F466\U0001F3FE"));
+  MoveCursorTo(model, model.text().length());
+  model.Backspace();
+#if defined(OS_MACOSX)
+  // On Mac, the entire emoji should be deleted to match NSTextField
+  // behavior.
+  EXPECT_EQ(base::WideToUTF16(L""), model.text());
+  EXPECT_EQ(0U, model.GetCursorPosition());
+#else
+  // https://crbug.com/829040
+  EXPECT_EQ(base::WideToUTF16(L"\U0001F466"), model.text());
+  EXPECT_EQ(2U, model.GetCursorPosition());
+#endif
 }
 
 TEST_F(TextfieldModelTest, EmptyString) {

ui/views/style/platform_style.cc

@@ -8,7 +8,9 @@
 #include "build/build_config.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/range/range.h"
 #include "ui/gfx/shadow_value.h"
+#include "ui/gfx/utf16_indexing.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/background.h"
 #include "ui/views/controls/button/label_button.h"
@@ -69,6 +71,15 @@ std::unique_ptr<ScrollBar> PlatformStyle::CreateScrollBar(bool is_horizontal) {
 // static
 void PlatformStyle::OnTextfieldEditFailed() {}
 
+// static
+gfx::Range PlatformStyle::RangeToDeleteBackwards(const base::string16& text,
+                                                 size_t cursor_position) {
+  // Delete one code point, which may be two UTF-16 words.
+  size_t previous_grapheme_index =
+      gfx::UTF16OffsetToIndex(text, cursor_position, -1);
+  return gfx::Range(cursor_position, previous_grapheme_index);
+}
+
 #endif  // OS_MACOSX
 
 #if !defined(DESKTOP_LINUX)

ui/views/style/platform_style.h

@@ -11,6 +11,10 @@
 #include "ui/views/controls/button/button.h"
 #include "ui/views/views_export.h"
 
+namespace gfx {
+class Range;
+}  // namespace gfx
+
 namespace views {
 
 class Border;
@@ -87,6 +91,14 @@ class VIEWS_EXPORT PlatformStyle {
   // the failed edit if platform-appropriate.
   static void OnTextfieldEditFailed();
 
+  // When deleting backwards in |string| with the cursor at index
+  // |cursor_position|, return the range of UTF-16 words to be deleted.
+  // This is to support deleting entire graphemes instead of individual
+  // characters when necessary on Mac, and code points made from surrogate
+  // pairs on other platforms.
+  static gfx::Range RangeToDeleteBackwards(const base::string16& text,
+                                           size_t cursor_position);
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(PlatformStyle);
 };

ui/views/style/platform_style_mac.mm

@@ -5,6 +5,7 @@
 #include "ui/views/style/platform_style.h"
 
 #include "base/memory/ptr_util.h"
+#include "base/strings/sys_string_conversions.h"
 #include "ui/base/ui_features.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/views/controls/button/label_button.h"
@@ -12,6 +13,24 @@
 
 #import <Cocoa/Cocoa.h>
 
+extern "C" {
+// From CFString private headers.
+typedef CF_ENUM(CFIndex, CFStringCharacterClusterType) {
+  kCFStringGraphemeCluster = 1, /* Unicode Grapheme Cluster */
+  kCFStringComposedCharacterCluster =
+      2, /* Compose all non-base (including spacing marks) */
+  kCFStringCursorMovementCluster =
+      3, /* Cluster suitable for cursor movements */
+  kCFStringBackwardDeletionCluster =
+      4 /* Cluster suitable for backward deletion */
+};
+
+CFRange CFStringGetRangeOfCharacterClusterAtIndex(
+    CFStringRef string,
+    CFIndex index,
+    CFStringCharacterClusterType type);
+}
+
 namespace views {
 
 const int PlatformStyle::kMinLabelButtonWidth = 32;
@@ -45,4 +64,23 @@ void PlatformStyle::OnTextfieldEditFailed() {
   NSBeep();
 }
 
+// static
+gfx::Range PlatformStyle::RangeToDeleteBackwards(const base::string16& text,
+                                                 size_t cursor_position) {
+  if (cursor_position == 0)
+    return gfx::Range();
+
+  base::ScopedCFTypeRef<CFStringRef> cf_string(
+      CFStringCreateWithCharactersNoCopy(kCFAllocatorDefault, text.data(),
+                                         text.size(), kCFAllocatorNull));
+  CFRange range_to_delete = CFStringGetRangeOfCharacterClusterAtIndex(
+      cf_string, cursor_position - 1, kCFStringBackwardDeletionCluster);
+
+  if (range_to_delete.location == NSNotFound)
+    return gfx::Range();
+
+  // The range needs to be reversed to undo correctly.
+  return gfx::Range(range_to_delete.location + range_to_delete.length,
+                    range_to_delete.location);
+}
 }  // namespace views