Re-factor two parameter version of endOfLine()

Source/core/editing/VisibleUnits.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 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 1011 matching lines @@
         if (!endTextBox->isLineBreak())
             endOffset += endTextBox->len();
         pos = Position(toText(endNode), endOffset);
     } else {
         pos = positionAfterNode(endNode);
     }
 
     return createVisiblePosition(pos, VP_UPSTREAM_IF_POSSIBLE);
 }
 
+// TODO(yosin) Rename this function to reflect the fact it ignores bidi levels.
+VisiblePosition endOfLine(const VisiblePosition& currentPosition)
+{
+    // TODO(yosin) this is the current behavior that might need to be fixed.
+    // Please refer to https://bugs.webkit.org/show_bug.cgi?id=49107 for detail.
+    VisiblePosition visPos = endPositionForLine(currentPosition, UseInlineBoxOrdering);
+
+    // Make sure the end of line is at the same line as the given input
+    // position. Else use the previous position to obtain end of line. This
+    // condition happens when the input position is before the space character
+    // at the end of a soft-wrapped non-editable line. In this scenario,
+    // |endPositionForLine()| would incorrectly hand back a position in the next
+    // line instead. This fix is to account for the discrepancy between lines
+    // with "webkit-line-break:after-white-space" style versus lines without
+    // that style, which would break before a space by default.
+    if (!inSameLine(currentPosition, visPos)) {
+        visPos = previousPositionOf(currentPosition);
+        if (visPos.isNull())
+            return VisiblePosition();
+        visPos = endPositionForLine(visPos, UseInlineBoxOrdering);
+    }
+
+    return honorEditingBoundaryAtOrAfter(visPos, currentPosition.deepEquivalent());
+}
+
 static bool inSameLogicalLine(const VisiblePosition& a, const VisiblePosition& b)
 {
     return a.isNotNull() && logicalStartOfLine(a).deepEquivalent() == logicalStartOfLine(b).deepEquivalent();
 }
 
-static VisiblePosition endOfLine(const VisiblePosition& c, LineEndpointComputationMode mode)
+VisiblePosition logicalEndOfLine(const VisiblePosition& currentPosition)
 {
-    // TODO: this is the current behavior that might need to be fixed.
+    // TODO(yosin) this is the current behavior that might need to be fixed.
     // Please refer to https://bugs.webkit.org/show_bug.cgi?id=49107 for detail.
-    VisiblePosition visPos = endPositionForLine(c, mode);
+    VisiblePosition visPos = endPositionForLine(currentPosition, UseLogicalOrdering);
 
-    if (mode == UseLogicalOrdering) {
-        // Make sure the end of line is at the same line as the given input position. For a wrapping line, the logical end
-        // position for the not-last-2-lines might incorrectly hand back the logical beginning of the next line.
-        // For example, <div contenteditable dir="rtl" style="line-break:before-white-space">abcdefg abcdefg abcdefg
-        // a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg </div>
-        // In this case, use the previous position of the computed logical end position.
-        if (!inSameLogicalLine(c, visPos))
-            visPos = previousPositionOf(visPos);
+    // Make sure the end of line is at the same line as the given input
+    // position. For a wrapping line, the logical end position for the
+    // not-last-2-lines might incorrectly hand back the logical beginning of the
+    // next line. For example,
+    // <div contenteditable dir="rtl" style="line-break:before-white-space">xyz
+    // a xyz xyz xyz xyz xyz xyz xyz xyz xyz xyz </div>

[hajimehoshi, 2015/09/09 10:59:09]

Why did you change this comment?

[yosin_UTC9, 2015/09/09 11:08:49]

to fit into 80 chars 
in previous line.

[hajimehoshi, 2015/09/09 11:11:26]

Hm, you could keep abcdefg, but that's OK to me

+    // In this case, use the previous position of the computed logical end
+    // position.
+    if (!inSameLogicalLine(currentPosition, visPos))
+        visPos = previousPositionOf(visPos);
 
-        if (ContainerNode* editableRoot = highestEditableRoot(c.deepEquivalent())) {
-            if (!editableRoot->contains(visPos.deepEquivalent().computeContainerNode()))
-                return createVisiblePosition(lastPositionInNode(editableRoot));
-        }
-
-        return honorEditingBoundaryAtOrAfter(visPos, c.deepEquivalent());
+    if (ContainerNode* editableRoot = highestEditableRoot(currentPosition.deepEquivalent())) {
+        if (!editableRoot->contains(visPos.deepEquivalent().computeContainerNode()))
+            return createVisiblePosition(lastPositionInNode(editableRoot));
     }
 
-    // Make sure the end of line is at the same line as the given input position. Else use the previous position to
-    // obtain end of line. This condition happens when the input position is before the space character at the end
-    // of a soft-wrapped non-editable line. In this scenario, endPositionForLine would incorrectly hand back a position
-    // in the next line instead. This fix is to account for the discrepancy between lines with webkit-line-break:after-white-space style
-    // versus lines without that style, which would break before a space by default.
-    if (!inSameLine(c, visPos)) {
-        visPos = previousPositionOf(c);
-        if (visPos.isNull())
-            return VisiblePosition();
-        visPos = endPositionForLine(visPos, UseInlineBoxOrdering);
-    }
-
-    return honorEditingBoundaryAtOrAfter(visPos, c.deepEquivalent());
-}
-
-// FIXME: Rename this function to reflect the fact it ignores bidi levels.
-VisiblePosition endOfLine(const VisiblePosition& currentPosition)
-{
-    return endOfLine(currentPosition, UseInlineBoxOrdering);
-}
-
-VisiblePosition logicalEndOfLine(const VisiblePosition& currentPosition)
-{
-    return endOfLine(currentPosition, UseLogicalOrdering);
+    return honorEditingBoundaryAtOrAfter(visPos, currentPosition.deepEquivalent());
 }
 
 template <typename Strategy>
 bool inSameLineAlgorithm(const PositionWithAffinityTemplate<Strategy>& position1, const PositionWithAffinityTemplate<Strategy>& position2)
 {
     if (position1.isNull() || position2.isNull())
         return false;
     PositionWithAffinityTemplate<Strategy> startOfLine1 = startOfLine(position1);
     PositionWithAffinityTemplate<Strategy> startOfLine2 = startOfLine(position2);
     if (startOfLine1 == startOfLine2)
@@ skipping 1932 matching lines @@
         return honorEditingBoundaryAtOrBefore(prev, visiblePosition.deepEquivalent());
     case CanSkipOverEditingBoundary:
         return skipToStartOfEditingBoundary(prev, visiblePosition.deepEquivalent());
     }
 
     ASSERT_NOT_REACHED();
     return honorEditingBoundaryAtOrBefore(prev, visiblePosition.deepEquivalent());
 }
 
 } // namespace blink