Use PositionMoveType::GraphemeBoundary

third_party/WebKit/Source/core/editing/EditingUtilities.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 46 matching lines @@
 #include "core/layout/LayoutTableCell.h"
 #include "wtf/Assertions.h"
 #include "wtf/StdLibExtras.h"
 #include "wtf/text/StringBuilder.h"
 #include "wtf/text/Unicode.h"
 
 namespace blink {
 
 using namespace HTMLNames;
 
+namespace {
+std::ostream& operator<<(std::ostream& os, PositionMoveType type)
+{
+    static const char* const texts[] = {
+        "CodeUnit", "CodePoint", "BackwardDeletion", "GraphemeCluster"
+    };
+    const auto& it = std::begin(texts) + static_cast<size_t>(type);
+    DCHECK_GE(it, std::begin(texts)) << "Unknown PositionMoveType value";
+    DCHECK_LT(it, std::end(texts)) << "Unknown PositionMoveType value";
+    return os << *it;
+}
+} // namespace
+
 // Atomic means that the node has no children, or has children which are ignored for the
 // purposes of editing.
 bool isAtomicNode(const Node *node)
 {
     return node && (!node->hasChildren() || editingIgnoresContent(node));
 }
 
 template <typename Traversal>
 static int comparePositions(Node* containerA, int offsetA, Node* containerB, int offsetB, bool* disconnected)
 {
@@ skipping 548 matching lines @@
         // There are two reasons child might be 0:
         //   1) The node is node like a text node that is not an element, and
         //      therefore has no children. Going backward one character at a
         //      time is correct.
         //   2) The old offset was a bogus offset like (<br>, 1), and there is
         //      no child. Going from 1 to 0 is correct.
         switch (moveType) {
         case PositionMoveType::CodeUnit:
             return PositionTemplate<Strategy>(node, offset - 1);
         case PositionMoveType::CodePoint:
-            // TODO(nona): Move to PositionMoveType::GraphemeBoundary case.
-            return PositionTemplate<Strategy>(node, previousGraphemeBoundaryOf(node, offset));
+            NOTIMPLEMENTED() << moveType << " is not yet supported.";

[yosin_UTC9, 2016/04/07 01:13:56]

Let's see results of layout test. If nothing is failed, we can get rid of
|PositionMoveType::CodePoint|. 

[Seigo Nonaka, 2016/04/08 05:47:45]

Done.

+            return PositionTemplate<Strategy>(node, offset - 1);
         case PositionMoveType::BackwardDeletion:
             return PositionTemplate<Strategy>(node, previousBackwardDeletionOffsetOf(node, offset));
+        case PositionMoveType::GraphemeCluster:
+            return PositionTemplate<Strategy>(node, previousGraphemeBoundaryOf(node, offset));
+        default:
+            NOTREACHED() << "Unhandled moveType: " << moveType;
         }
     }
 
     if (ContainerNode* parent = Strategy::parent(*node)) {
         if (editingIgnoresContent(parent))
             return PositionTemplate<Strategy>::beforeNode(parent);
         // TODO(yosin) We should use |Strategy::index(Node&)| instead of
         // |Node::nodeIndex()|.
         return PositionTemplate<Strategy>(parent, node->nodeIndex());
     }
@@ skipping 26 matching lines @@
 
     // TODO(yosin) We should use |Strategy::lastOffsetForEditing()| instead of
     // DOM tree version.
     if (!Strategy::hasChildren(*node) && offset < EditingStrategy::lastOffsetForEditing(node)) {
         // There are two reasons child might be 0:
         //   1) The node is node like a text node that is not an element, and
         //      therefore has no children. Going forward one character at a time
         //      is correct.
         //   2) The new offset is a bogus offset like (<br>, 1), and there is no
         //      child. Going from 0 to 1 is correct.
-        // TODO(nona): Call nextGraphemeBoundaryOf if
-        //             moveType == PositionMoveType::GraphemeBoundary
-        return PositionTemplate<Strategy>::editingPositionOf(node, (moveType == PositionMoveType::CodePoint) ? nextGraphemeBoundaryOf(node, offset) : offset + 1);
+        switch (moveType) {
+        case PositionMoveType::CodeUnit:
+            return PositionTemplate<Strategy>::editingPositionOf(node, offset + 1);
+        case PositionMoveType::CodePoint:

[yosin_UTC9, 2016/04/08 00:51:57]

Since, nobody use |PositionMoveType::CodePoint|, could you get rid of this
rather than |NOTIMPLEMENTED()|?

[Seigo Nonaka, 2016/04/08 05:47:45]

Done.

+            NOTIMPLEMENTED() << moveType << " is not yet supported.";
+            return PositionTemplate<Strategy>::editingPositionOf(node, offset + 1);
+        case PositionMoveType::BackwardDeletion:
+            NOTREACHED()
+                << "BackwardDeletion is only available for prevPositionOf "
+                << "functions.";
+            return PositionTemplate<Strategy>::editingPositionOf(node, offset + 1);
+        case PositionMoveType::GraphemeCluster:
+            return PositionTemplate<Strategy>::editingPositionOf(node, nextGraphemeBoundaryOf(node, offset));
+        default:
+            NOTREACHED() << "Unhandled moveType: " << moveType;
+        }
     }
 
     if (ContainerNode* parent = Strategy::parent(*node))
         return PositionTemplate<Strategy>::editingPositionOf(parent, Strategy::index(*node) + 1);
     return position;
 }
 
 Position nextPositionOf(const Position& position, PositionMoveType moveType)
 {
     return nextPositionOfAlgorithm<EditingStrategy>(position, moveType);
@@ skipping 1002 matching lines @@
     // instead of possibly at the end of the last node before the selection
     return mostForwardCaretPosition(visiblePosition.deepEquivalent());
 }
 
 bool isTextSecurityNode(const Node* node)
 {
     return node && node->layoutObject() && node->layoutObject()->style()->textSecurity() != TSNONE;
 }
 
 } // namespace blink