Incorrect caret position in empty elements with a vertical writing mode

https://bugs.webkit.org/show_bug.cgi?id=248322 rdar://102652127 Reviewed by Tim Nguyen. Caret rect computation for empty elements does not fully account for vertical writing mode. Fix by consistently using logical values. * LayoutTests/editing/selection/caret-in-empty-block-vertical-horizontal-rtl-text-indent-padding-expected.txt: Added. * LayoutTests/editing/selection/caret-in-empty-block-vertical-horizontal-rtl-text-indent-padding.html: Added. * LayoutTests/editing/selection/caret-in-empty-block-vertical-rl-rtl-expected.txt: Added. * LayoutTests/editing/selection/caret-in-empty-block-vertical-rl-rtl.html: Added. * LayoutTests/editing/selection/caret-in-empty-inline-vertical-rl-rtl-expected.txt: Added. * LayoutTests/editing/selection/caret-in-empty-inline-vertical-rl-rtl.html: Added. * Source/WebCore/rendering/CaretRectComputation.cpp: (WebCore::computeCaretRectForEmptyElement): (WebCore::computeCaretRectForBlock): (WebCore::computeCaretRectForInline): * Source/WebCore/rendering/RenderBoxModelObject.h: * Source/WebCore/rendering/RenderBoxModelObjectInlines.h: (WebCore::RenderBoxModelObject::borderAndPaddingLogicalHeight const): (WebCore::RenderBoxModelObject::borderAndPaddingLogicalLeft const): (WebCore::RenderBoxModelObject::borderAndPaddingLogicalRight const): (WebCore::RenderBoxModelObject::borderAndPaddingEnd const): Canonical link: https://commits.webkit.org/270707@main
WebKit · Nov 14, 2023 · 3931fce · 3931fce
1 parent 748284b
commit 3931fce
Show file tree

Hide file tree

Showing 9 changed files with 190 additions and 8 deletions.
diff --git a/...g/selection/caret-in-empty-block-vertical-horizontal-rtl-text-indent-padding-expected.txt b/...g/selection/caret-in-empty-block-vertical-horizontal-rtl-text-indent-padding-expected.txt
@@ -0,0 +1,11 @@
+This test verifies that an empty block contenteditable element with indented text and padding, and right-to-left text direction, gets a valid caret rect.
+
+
+PASS caretRect.left is correct.
+PASS caretRect.top is correct.
+PASS caretRect.width is correct.
+PASS caretRect.height is correct.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/...s/editing/selection/caret-in-empty-block-vertical-horizontal-rtl-text-indent-padding.html b/...s/editing/selection/caret-in-empty-block-vertical-horizontal-rtl-text-indent-padding.html
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js" type="text/javascript"></script>
+<style>
+
+.hidden {
+    display: none;
+}
+
+#text, #emptyBlock {
+    border: 1px solid black;
+    inline-size: 100px;
+    text-indent: 20px;
+    padding: 10px 20px 30px 40px;
+    direction: rtl;
+}
+</style>
+</head>
+<body>
+<p>This test verifies that an empty block contenteditable element with indented text and padding, and right-to-left text direction, gets a valid caret rect.</p>
+<div id="text" contenteditable>Some text.</div>
+<div id="emptyBlock" contenteditable></div><br>
+<div id="console"></div>
+</body>
+<script>
+    var text = document.getElementById("text");
+    var emptyBlock = document.getElementById("emptyBlock");
+
+    if (window.internals) {
+        emptyBlock.classList.toggle("hidden");
+        getSelection().collapse(text, 0);
+        var textCaretRect = internals.absoluteCaretBounds();
+
+        text.classList.toggle("hidden");
+        emptyBlock.classList.toggle("hidden");
+
+        getSelection().collapse(emptyBlock, 0);
+        var emptyBlockCaretRect = internals.absoluteCaretBounds();
+
+        ['left', 'top', 'width', 'height'].forEach((property) => {
+            if (Math.abs(emptyBlockCaretRect[property] - textCaretRect[property]) <= 1)
+                testPassed(`caretRect.${property} is correct.`);
+            else
+                testFailed(`caretRect.${property} is incorrect.`);
+        });
+    }
+</script>
+</html>
diff --git a/LayoutTests/editing/selection/caret-in-empty-block-vertical-rl-rtl-expected.txt b/LayoutTests/editing/selection/caret-in-empty-block-vertical-rl-rtl-expected.txt
@@ -0,0 +1,11 @@
+This test verifies that an empty block contenteditable element with a vertical writing mode, and right-to-left text direction, gets a valid caret rect.
+
+
+PASS caretRect.left is correct.
+PASS caretRect.top is correct.
+PASS caretRect.width is correct.
+PASS caretRect.height is correct.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/selection/caret-in-empty-block-vertical-rl-rtl.html b/LayoutTests/editing/selection/caret-in-empty-block-vertical-rl-rtl.html
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js" type="text/javascript"></script>
+<style>
+
+.hidden {
+    display: none;
+}
+
+#text, #emptyBlock {
+    border: 1px solid black;
+    inline-size: 100px;
+    writing-mode: vertical-rl;
+    direction: rtl;
+}
+</style>
+</head>
+<body>
+<p>This test verifies that an empty block contenteditable element with a vertical writing mode, and right-to-left text direction, gets a valid caret rect.</p>
+<div id="text" contenteditable>Some text.</div>
+<div id="emptyBlock" contenteditable></div><br>
+<div id="console"></div>
+</body>
+<script>
+    var text = document.getElementById("text");
+    var emptyBlock = document.getElementById("emptyBlock");
+
+    if (window.internals) {
+        emptyBlock.classList.toggle("hidden");
+        getSelection().collapse(text, 0);
+        var textCaretRect = internals.absoluteCaretBounds();
+
+        text.classList.toggle("hidden");
+        emptyBlock.classList.toggle("hidden");
+
+        getSelection().collapse(emptyBlock, 0);
+        var emptyBlockCaretRect = internals.absoluteCaretBounds();
+
+        ['left', 'top', 'width', 'height'].forEach((property) => {
+            if (Math.abs(emptyBlockCaretRect[property] - textCaretRect[property]) <= 1)
+                testPassed(`caretRect.${property} is correct.`);
+            else
+                testFailed(`caretRect.${property} is incorrect.`);
+        });
+    }
+</script>
+</html>
diff --git a/LayoutTests/editing/selection/caret-in-empty-inline-vertical-rl-rtl-expected.txt b/LayoutTests/editing/selection/caret-in-empty-inline-vertical-rl-rtl-expected.txt
@@ -0,0 +1,11 @@
+This test verifies that an empty inline contenteditable element with a vertical writing mode, and right-to-left text direction, gets a valid caret rect.
+
+
+PASS caretRect.left is correct.
+PASS caretRect.top is correct.
+PASS caretRect.width is correct.
+PASS caretRect.height is correct.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/selection/caret-in-empty-inline-vertical-rl-rtl.html b/LayoutTests/editing/selection/caret-in-empty-inline-vertical-rl-rtl.html
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js" type="text/javascript"></script>
+<style>
+
+.hidden {
+    display: none;
+}
+
+#text, #emptyInline {
+    border: 1px solid black;
+    inline-size: 100px;
+    writing-mode: vertical-rl;
+    direction: rtl;
+}
+</style>
+</head>
+<body>
+<p>This test verifies that an empty inline contenteditable element with a vertical writing mode, and right-to-left text direction, gets a valid caret rect.</p>
+<div id="text" contenteditable>Some text.</div>
+<div id="emptyInline" contenteditable></div><br>
+<div id="console"></div>
+</body>
+<script>
+    var text = document.getElementById("text");
+    var emptyInline = document.getElementById("emptyInline");
+
+    if (window.internals) {
+        emptyInline.classList.toggle("hidden");
+        getSelection().collapse(text, 0);
+        var textCaretRect = internals.absoluteCaretBounds();
+
+        text.classList.toggle("hidden");
+        emptyInline.classList.toggle("hidden");
+
+        getSelection().collapse(emptyInline, 0);
+        var emptyInlineCaretRect = internals.absoluteCaretBounds();
+
+        ['left', 'top', 'width', 'height'].forEach((property) => {
+            if (Math.abs(emptyInlineCaretRect[property] - textCaretRect[property]) <= 1)
+                testPassed(`caretRect.${property} is correct.`);
+            else
+                testFailed(`caretRect.${property} is incorrect.`);
+        });
+    }
+</script>
+</html>
diff --git a/Source/WebCore/rendering/CaretRectComputation.cpp b/Source/WebCore/rendering/CaretRectComputation.cpp
@@ -54,7 +54,7 @@ int caretWidth()
 #endif
 }
 
-static LayoutRect computeCaretRectForEmptyElement(const RenderBoxModelObject& renderer, LayoutUnit width, LayoutUnit textIndentOffset, CaretRectMode caretRectMode)
+static LayoutRect computeCaretRectForEmptyElement(const RenderBoxModelObject& renderer, LayoutUnit logicalWidth, LayoutUnit textIndentOffset, CaretRectMode caretRectMode)
 {
     ASSERT(!renderer.firstChild() || renderer.firstChild()->isPseudoElement());
 
@@ -91,8 +91,8 @@ static LayoutRect computeCaretRectForEmptyElement(const RenderBoxModelObject& re
         break;
     }
 
-    LayoutUnit x = renderer.borderLeft() + renderer.paddingLeft();
-    LayoutUnit maxX = width - renderer.borderRight() - renderer.paddingRight();
+    LayoutUnit x = renderer.borderAndPaddingLogicalLeft();
+    LayoutUnit maxX = logicalWidth - renderer.borderAndPaddingLogicalRight();
 
     switch (alignment) {
     case AlignLeft:
@@ -116,12 +116,12 @@ static LayoutRect computeCaretRectForEmptyElement(const RenderBoxModelObject& re
 
     auto lineHeight = renderer.lineHeight(true, currentStyle.isHorizontalWritingMode() ? HorizontalLine : VerticalLine, PositionOfInteriorLineBoxes);
     auto height = std::min(lineHeight, LayoutUnit { currentStyle.metricsOfPrimaryFont().height() });
-    auto y = renderer.paddingTop() + renderer.borderTop() + (lineHeight > height ? (lineHeight - height) / 2 : LayoutUnit { });
+    auto y = renderer.borderAndPaddingBefore() + (lineHeight > height ? (lineHeight - height) / 2 : LayoutUnit { });
 
     auto rect = LayoutRect(x, y, caretWidth(), height);
 
     if (caretRectMode == CaretRectMode::ExpandToEndOfLine)
-        rect.shiftMaxXEdgeTo(width);
+        rect.shiftMaxXEdgeTo(logicalWidth);
 
     return currentStyle.isHorizontalWritingMode() ? rect : rect.transposedRect();
 }
@@ -302,7 +302,7 @@ static LayoutRect computeCaretRectForBlock(const RenderBlock& renderer, const In
     if (renderer.firstChild() && !renderer.firstChild()->isPseudoElement())
         return computeCaretRectForBox(renderer, boxAndOffset, caretRectMode);
 
-    return computeCaretRectForEmptyElement(renderer, renderer.width(), renderer.textIndentOffset(), caretRectMode);
+    return computeCaretRectForEmptyElement(renderer, renderer.logicalWidth(), renderer.textIndentOffset(), caretRectMode);
 }
 
 static LayoutRect computeCaretRectForInline(const RenderInline& renderer)
@@ -316,7 +316,7 @@ static LayoutRect computeCaretRectForInline(const RenderInline& renderer)
         return { };
     }
 
-    LayoutRect caretRect = computeCaretRectForEmptyElement(renderer, renderer.horizontalBorderAndPaddingExtent(), 0, CaretRectMode::Normal);
+    LayoutRect caretRect = computeCaretRectForEmptyElement(renderer, renderer.borderAndPaddingLogicalWidth(), 0, CaretRectMode::Normal);
 
     if (auto firstInlineBox = InlineIterator::firstInlineBoxFor(renderer))
         caretRect.moveBy(LayoutPoint { firstInlineBox->visualRectIgnoringBlockDirection().location() });

diff --git a/Source/WebCore/rendering/RenderBoxModelObject.h b/Source/WebCore/rendering/RenderBoxModelObject.h
@@ -138,6 +138,7 @@ class RenderBoxModelObject : public RenderLayerModelObject {
     virtual inline LayoutUnit borderEnd() const;
 
     inline LayoutUnit borderAndPaddingStart() const;
+    inline LayoutUnit borderAndPaddingEnd() const;
     inline LayoutUnit borderAndPaddingBefore() const;
     inline LayoutUnit borderAndPaddingAfter() const;
 
@@ -151,6 +152,7 @@ class RenderBoxModelObject : public RenderLayerModelObject {
     inline LayoutUnit borderAndPaddingLogicalHeight() const;
     inline LayoutUnit borderAndPaddingLogicalWidth() const;
     inline LayoutUnit borderAndPaddingLogicalLeft() const;
+    inline LayoutUnit borderAndPaddingLogicalRight() const;
 
     inline LayoutUnit borderLogicalLeft() const;
     inline LayoutUnit borderLogicalRight() const;

diff --git a/Source/WebCore/rendering/RenderBoxModelObjectInlines.h b/Source/WebCore/rendering/RenderBoxModelObjectInlines.h
@@ -29,9 +29,11 @@ inline LayoutUnit RenderBoxModelObject::borderAfter() const { return LayoutUnit(
 inline LayoutUnit RenderBoxModelObject::borderAndPaddingAfter() const { return borderAfter() + paddingAfter(); }
 inline LayoutUnit RenderBoxModelObject::borderAndPaddingBefore() const { return borderBefore() + paddingBefore(); }
 inline LayoutUnit RenderBoxModelObject::borderAndPaddingLogicalHeight() const { return borderAndPaddingBefore() + borderAndPaddingAfter(); }
-inline LayoutUnit RenderBoxModelObject::borderAndPaddingLogicalLeft() const { return style().isHorizontalWritingMode() ? borderLeft() + paddingLeft() : borderTop() + paddingTop(); }
 inline LayoutUnit RenderBoxModelObject::borderAndPaddingLogicalWidth() const { return borderStart() + borderEnd() + paddingStart() + paddingEnd(); }
+inline LayoutUnit RenderBoxModelObject::borderAndPaddingLogicalLeft() const { return style().isHorizontalWritingMode() ? borderLeft() + paddingLeft() : borderTop() + paddingTop(); }
+inline LayoutUnit RenderBoxModelObject::borderAndPaddingLogicalRight() const { return style().isHorizontalWritingMode() ? borderRight() + paddingRight() : borderBottom() + paddingBottom(); }
 inline LayoutUnit RenderBoxModelObject::borderAndPaddingStart() const { return borderStart() + paddingStart(); }
+inline LayoutUnit RenderBoxModelObject::borderAndPaddingEnd() const { return borderEnd() + paddingEnd(); }
 inline LayoutUnit RenderBoxModelObject::borderBefore() const { return LayoutUnit(style().borderBeforeWidth()); }
 inline LayoutUnit RenderBoxModelObject::borderBottom() const { return LayoutUnit(style().borderBottomWidth()); }
 inline LayoutUnit RenderBoxModelObject::borderEnd() const { return LayoutUnit(style().borderEndWidth()); }