Fix caret move by line when padding-top is set

Fix caret move by line when padding-top is set https://bugs.webkit.org/show_bug.cgi?id=248413 rdar://problem/102991672 Reviewed by Ryosuke Niwa. This patch is to align WebKit behavior with Gecko / Firefox and Blink / Chromium. Partial Merge - https://chromium.googlesource.com/chromium/blink/+/87805d6f9313997171b970d847e141a18b95dcf7 Moving carets by line, either forward or backward, could be prevented because the large part of the relevant code path for that case (previousLinePosition(), nextLinePosition(), etc.) uses int and IntPoint. This patch fixes all relevant functions to use LayoutUnit and LayoutPoint in VisibleUnits. * Source/WebCore/editing/VisibleUnits.cpp: (absoluteLineDirectionPointToLocalPointInBlock): Modify "IntPoint" and "int" to "LayoutPoint" and "LayoutUnit" (previousLinePosition): Ditto (nextLinePosition): Ditto (previousParagraphPosition): Ditto (nextParagraphPosition): Ditto * Source/WebCore/editing/VisibleUnits.h: Introduce 'LayoutUnit' class and use it in similar functions as our changes in .cpp file * LayoutTests/editing/selection/move-with-padding-top.html: Add Test Case * LayoutTests/editing/selection/move-with-padding-top-expected.txt: Add Test Case Expectation Canonical link: https://commits.webkit.org/259906@main
WebKit · Feb 6, 2023 · 7e9c271 · 7e9c271
1 parent 47f7b57
commit 7e9c271
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 12 deletions.
diff --git a/LayoutTests/editing/selection/move-with-padding-top-expected.txt b/LayoutTests/editing/selection/move-with-padding-top-expected.txt
@@ -0,0 +1,5 @@
+
+PASS padding-top=0
+PASS padding-top=4pt
+PASS padding-top=4.8pt
+
diff --git a/LayoutTests/editing/selection/move-with-padding-top.html b/LayoutTests/editing/selection/move-with-padding-top.html
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<style>
+html, body {
+    margin: 0;
+}
+#container {
+    width:300px;
+}
+</style>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<div id="container" contenteditable="true">
+    <p><span style="font-size:24px">Caret navigation using up arrow key does not work</span></p>
+</div>
+<script>
+var p = document.getElementsByTagName("p")[0];
+var selection = window.getSelection();
+
+testMoveByLineWithPaddingTop("0");
+testMoveByLineWithPaddingTop("4pt");
+testMoveByLineWithPaddingTop("4.8pt");
+
+function testMoveByLineWithPaddingTop(paddingTop) {
+    test(function () {
+        p.style.paddingTop = paddingTop;
+        var textNode = document.getElementsByTagName("span")[0].firstChild;
+        selection.collapse(textNode, 1); // avoid line-top not to be bothered by affinity
+        var beforeRect = selection.getRangeAt(0).getClientRects()[0];
+
+        selection.modify("move", "forward", "line");
+        var forwardRect = selection.getRangeAt(0).getClientRects()[0];
+        assert_greater_than(forwardRect.top, beforeRect.top, "move forward by line");
+
+        selection.modify("move", "backward", "line");
+        var backwardRect = selection.getRangeAt(0).getClientRects()[0];
+        assert_equals(backwardRect.top, beforeRect.top, "move backward by line");
+    }, "padding-top=" + paddingTop);
+}
+
+if (window.testRunner)
+    container.style.display = "none";
+</script>
diff --git a/Source/WebCore/editing/VisibleUnits.cpp b/Source/WebCore/editing/VisibleUnits.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004-2022 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2023 Apple Inc. All rights reserved.
  * Copyright (C) 2013-2015 Google Inc. All rights reserved.
  * 
  * Redistribution and use in source and binary forms, with or without
@@ -937,15 +937,15 @@ bool isLogicalEndOfLine(const VisiblePosition& p)
     return p.isNotNull() && p == logicalEndOfLine(p);
 }
 
-static inline IntPoint absoluteLineDirectionPointToLocalPointInBlock(InlineIterator::LineBoxIterator& lineBox, int lineDirectionPoint)
+static inline LayoutPoint absoluteLineDirectionPointToLocalPointInBlock(InlineIterator::LineBoxIterator& lineBox, LayoutUnit lineDirectionPoint)
 {
     auto& root = lineBox->formattingContextRoot();
     auto absoluteBlockPoint = root.localToAbsolute(FloatPoint()) - toFloatSize(root.scrollPosition());
 
     if (root.isHorizontalWritingMode())
-        return IntPoint(lineDirectionPoint - absoluteBlockPoint.x(), contentStartInBlockDirection(*lineBox));
+        return LayoutPoint(lineDirectionPoint - absoluteBlockPoint.x(), contentStartInBlockDirection(*lineBox));
 
-    return IntPoint(contentStartInBlockDirection(*lineBox), lineDirectionPoint - absoluteBlockPoint.y());
+    return LayoutPoint(contentStartInBlockDirection(*lineBox), lineDirectionPoint - absoluteBlockPoint.y());
 }
 
 static Element* rootEditableOrDocumentElement(Node& node, EditableType editableType)
@@ -955,7 +955,7 @@ static Element* rootEditableOrDocumentElement(Node& node, EditableType editableT
     return node.document().documentElement();
 }
 
-VisiblePosition previousLinePosition(const VisiblePosition& visiblePosition, int lineDirectionPoint, EditableType editableType)
+VisiblePosition previousLinePosition(const VisiblePosition& visiblePosition, LayoutUnit lineDirectionPoint, EditableType editableType)
 {
     Position p = visiblePosition.deepEquivalent();
     Node* node = p.deprecatedNode();
@@ -1010,7 +1010,7 @@ VisiblePosition previousLinePosition(const VisiblePosition& visiblePosition, int
     return firstPositionInNode(rootElement);
 }
 
-VisiblePosition nextLinePosition(const VisiblePosition& visiblePosition, int lineDirectionPoint, EditableType editableType)
+VisiblePosition nextLinePosition(const VisiblePosition& visiblePosition, LayoutUnit lineDirectionPoint, EditableType editableType)
 {
     Position p = visiblePosition.deepEquivalent();
     Node* node = p.deprecatedNode();
@@ -1321,7 +1321,7 @@ bool isBlankParagraph(const VisiblePosition& position)
     return isStartOfParagraph(position) && startOfParagraph(position.next()) != startOfParagraph(position);
 }
 
-VisiblePosition previousParagraphPosition(const VisiblePosition& p, int x)
+VisiblePosition previousParagraphPosition(const VisiblePosition& p, LayoutUnit x)
 {
     VisiblePosition pos = p;
     do {
@@ -1333,7 +1333,7 @@ VisiblePosition previousParagraphPosition(const VisiblePosition& p, int x)
     return pos;
 }
 
-VisiblePosition nextParagraphPosition(const VisiblePosition& p, int x)
+VisiblePosition nextParagraphPosition(const VisiblePosition& p, LayoutUnit x)
 {
     VisiblePosition pos = p;
     do {

diff --git a/Source/WebCore/editing/VisibleUnits.h b/Source/WebCore/editing/VisibleUnits.h
@@ -31,6 +31,7 @@
 namespace WebCore {
 
 class Node;
+class LayoutUnit;
 class VisiblePosition;
 class SimplifiedBackwardsTextIterator;
 class TextIterator;
@@ -55,8 +56,8 @@ WEBCORE_EXPORT VisiblePosition nextSentencePosition(const VisiblePosition&);
 // lines
 WEBCORE_EXPORT VisiblePosition startOfLine(const VisiblePosition&);
 WEBCORE_EXPORT VisiblePosition endOfLine(const VisiblePosition&);
-WEBCORE_EXPORT VisiblePosition previousLinePosition(const VisiblePosition&, int lineDirectionPoint, EditableType = ContentIsEditable);
-WEBCORE_EXPORT VisiblePosition nextLinePosition(const VisiblePosition&, int lineDirectionPoint, EditableType = ContentIsEditable);
+WEBCORE_EXPORT VisiblePosition previousLinePosition(const VisiblePosition&, LayoutUnit lineDirectionPoint, EditableType = ContentIsEditable);
+WEBCORE_EXPORT VisiblePosition nextLinePosition(const VisiblePosition&, LayoutUnit lineDirectionPoint, EditableType = ContentIsEditable);
 WEBCORE_EXPORT bool inSameLine(const VisiblePosition&, const VisiblePosition&);
 WEBCORE_EXPORT bool isStartOfLine(const VisiblePosition&);
 WEBCORE_EXPORT bool isEndOfLine(const VisiblePosition&);
@@ -70,8 +71,8 @@ VisiblePosition rightBoundaryOfLine(const VisiblePosition&, TextDirection, bool*
 WEBCORE_EXPORT VisiblePosition startOfParagraph(const VisiblePosition&, EditingBoundaryCrossingRule = CannotCrossEditingBoundary);
 WEBCORE_EXPORT VisiblePosition endOfParagraph(const VisiblePosition&, EditingBoundaryCrossingRule = CannotCrossEditingBoundary);
 VisiblePosition startOfNextParagraph(const VisiblePosition&);
-WEBCORE_EXPORT VisiblePosition previousParagraphPosition(const VisiblePosition&, int x);
-WEBCORE_EXPORT VisiblePosition nextParagraphPosition(const VisiblePosition&, int x);
+WEBCORE_EXPORT VisiblePosition previousParagraphPosition(const VisiblePosition&, LayoutUnit x);
+WEBCORE_EXPORT VisiblePosition nextParagraphPosition(const VisiblePosition&, LayoutUnit x);
 WEBCORE_EXPORT bool isStartOfParagraph(const VisiblePosition&, EditingBoundaryCrossingRule = CannotCrossEditingBoundary);
 WEBCORE_EXPORT bool isEndOfParagraph(const VisiblePosition&, EditingBoundaryCrossingRule = CannotCrossEditingBoundary);
 bool inSameParagraph(const VisiblePosition&, const VisiblePosition&, EditingBoundaryCrossingRule = CannotCrossEditingBoundary);