Title: [262525] trunk
Revision
262525
Author
[email protected]
Date
2020-06-03 17:01:55 -0700 (Wed, 03 Jun 2020)

Log Message

Inserted text placeholder should vertically align to top and behave like block-level element when it has 0 width
https://bugs.webkit.org/show_bug.cgi?id=212716
<rdar://problem/62672479>

Reviewed by Darin Adler.

Source/WebCore:

Refine the appearance of a text placeholder based on feedback:
    1. If the width of the placeholder is 0 then put it on its own line. This is accomplished by making it
       CSS "display: block".
    2. Vertically align the placeholder with the top of the line.

Both of these refinements are to make the rendering more like TextKit's rendering.

Tests: editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height.html
       editing/text-placeholder/insert-into-content-editable-zero-width.html

* html/shadow/TextPlaceholderElement.cpp:

LayoutTests:

Add tests.

* editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height-expected.html: Added.
* editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height.html: Added.
* editing/text-placeholder/insert-into-content-editable-zero-width-expected.html: Added.
* editing/text-placeholder/insert-into-content-editable-zero-width.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (262524 => 262525)


--- trunk/LayoutTests/ChangeLog	2020-06-03 23:22:39 UTC (rev 262524)
+++ trunk/LayoutTests/ChangeLog	2020-06-04 00:01:55 UTC (rev 262525)
@@ -1,3 +1,18 @@
+2020-06-03  Daniel Bates  <[email protected]>
+
+        Inserted text placeholder should vertically align to top and behave like block-level element when it has 0 width
+        https://bugs.webkit.org/show_bug.cgi?id=212716
+        <rdar://problem/62672479>
+
+        Reviewed by Darin Adler.
+
+        Add tests.
+
+        * editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height-expected.html: Added.
+        * editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height.html: Added.
+        * editing/text-placeholder/insert-into-content-editable-zero-width-expected.html: Added.
+        * editing/text-placeholder/insert-into-content-editable-zero-width.html: Added.
+
 2020-06-03  Pinki Gyanchandani  <[email protected]>
 
         Release Assert @ WebCore::RenderTreeBuilder::RenderTreeBuilder

Added: trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height-expected.html (0 => 262525)


--- trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height-expected.html	                        (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height-expected.html	2020-06-04 00:01:55 UTC (rev 262525)
@@ -0,0 +1,12 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="initial-scale=1.0">
+<script src=""
+<link rel="stylesheet" href=""
+</head>
+<body>
+<p>This tests that a text placeholder with non-zero width and height is inserted into a <code>contenteditable</code> element. This test can only be run in WebKitTestRunner or DumpRenderTree.</p>
+<div id="test" class="test" style="width: 200px; height: 200px; border: 1px solid black" contenteditable="true">Hello<div style="display:inline-block; vertical-align: top; margin: 0; padding: 0; height: 3em; width: 1em;">&nbsp;</div>from Cupertino!</div>
+</body>
+</html>

Added: trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height.html (0 => 262525)


--- trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height.html	                        (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height.html	2020-06-04 00:01:55 UTC (rev 262525)
@@ -0,0 +1,41 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="initial-scale=1.0">
+<script src=""
+<link rel="stylesheet" href=""
+</head>
+<body>
+<p>This tests that a text placeholder with non-zero width and height is inserted into a <code>contenteditable</code> element. This test can only be run in WebKitTestRunner or DumpRenderTree.</p>
+<div id="test" class="test" style="width: 200px; height: 200px; border: 1px solid black" contenteditable="true">Hello from Cupertino!</div>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+async function runTest()
+{
+    let testElement = document.getElementById("test");
+    if (window.testRunner)
+        await UIHelper.activateElementAndWaitForInputSession(testElement);
+    else
+        await UIHelper.callFunctionAndWaitForEvent(() => testElement.focus(), testElement, "focus");
+
+    let positionOfFrom = testElement.textContent.indexOf("from");
+    window.getSelection().setBaseAndExtent(testElement.firstChild, positionOfFrom, testElement.firstChild, positionOfFrom);
+
+    if (window.internals) {
+        // Because we are using the Ahem font almost all glyphs have the same width.
+        let glyphWidth = parseInt(window.getComputedStyle(testElement, null).fontSize, 10);
+        internals.insertTextPlaceholder(glyphWidth, 3 * glyphWidth);
+    }
+
+    // Defocus field to hide the cursor.
+    await UIHelper.callFunctionAndWaitForEvent(() => testElement.blur(), testElement, "blur");
+
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+runTest();
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-zero-width-expected.html (0 => 262525)


--- trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-zero-width-expected.html	                        (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-zero-width-expected.html	2020-06-04 00:01:55 UTC (rev 262525)
@@ -0,0 +1,12 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="initial-scale=1.0">
+<script src=""
+<link rel="stylesheet" href=""
+</head>
+<body>
+<p>This tests that a text placeholder with 0 width is inserted into a <code>contenteditable</code> element. This test can only be run in WebKitTestRunner or DumpRenderTree.</p>
+<div id="test" class="test" style="width: 200px; height: 200px; border: 1px solid black" contenteditable="true">Hello<p style="height: 1em; margin: 0; padding: 0">&nbsp;</p>from Cupertino!</div>
+</body>
+</html>

Added: trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-zero-width.html (0 => 262525)


--- trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-zero-width.html	                        (rev 0)
+++ trunk/LayoutTests/editing/text-placeholder/insert-into-content-editable-zero-width.html	2020-06-04 00:01:55 UTC (rev 262525)
@@ -0,0 +1,40 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="initial-scale=1.0">
+<script src=""
+<link rel="stylesheet" href=""
+</head>
+<body>
+<p>This tests that a text placeholder with 0 width is inserted into a <code>contenteditable</code> element. This test can only be run in WebKitTestRunner or DumpRenderTree.</p>
+<div id="test" class="test" style="width: 200px; height: 200px; border: 1px solid black" contenteditable="true">Hello from Cupertino!</div>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+async function runTest()
+{
+    let testElement = document.getElementById("test");
+    if (window.testRunner)
+        await UIHelper.activateElementAndWaitForInputSession(testElement);
+    else
+        await UIHelper.callFunctionAndWaitForEvent(() => testElement.focus(), testElement, "focus");
+
+    let positionOfFrom = testElement.textContent.indexOf("from");
+    window.getSelection().setBaseAndExtent(testElement.firstChild, positionOfFrom, testElement.firstChild, positionOfFrom);
+
+    if (window.internals) {
+        let lineHeight = parseInt(window.getComputedStyle(testElement, null).lineHeight, 10);
+        internals.insertTextPlaceholder(0 /* width */, lineHeight);
+    }
+
+    // Defocus field to hide the cursor.
+    await UIHelper.callFunctionAndWaitForEvent(() => testElement.blur(), testElement, "blur");
+
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+runTest();
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (262524 => 262525)


--- trunk/Source/WebCore/ChangeLog	2020-06-03 23:22:39 UTC (rev 262524)
+++ trunk/Source/WebCore/ChangeLog	2020-06-04 00:01:55 UTC (rev 262525)
@@ -1,3 +1,23 @@
+2020-06-03  Daniel Bates  <[email protected]>
+
+        Inserted text placeholder should vertically align to top and behave like block-level element when it has 0 width
+        https://bugs.webkit.org/show_bug.cgi?id=212716
+        <rdar://problem/62672479>
+
+        Reviewed by Darin Adler.
+
+        Refine the appearance of a text placeholder based on feedback:
+            1. If the width of the placeholder is 0 then put it on its own line. This is accomplished by making it
+               CSS "display: block".
+            2. Vertically align the placeholder with the top of the line.
+
+        Both of these refinements are to make the rendering more like TextKit's rendering.
+
+        Tests: editing/text-placeholder/insert-into-content-editable-non-zero-width-and-height.html
+               editing/text-placeholder/insert-into-content-editable-zero-width.html
+
+        * html/shadow/TextPlaceholderElement.cpp:
+
 2020-06-03  Pinki Gyanchandani  <[email protected]>
 
         Release Assert @ WebCore::RenderTreeBuilder::RenderTreeBuilder

Modified: trunk/Source/WebCore/html/shadow/TextPlaceholderElement.cpp (262524 => 262525)


--- trunk/Source/WebCore/html/shadow/TextPlaceholderElement.cpp	2020-06-03 23:22:39 UTC (rev 262524)
+++ trunk/Source/WebCore/html/shadow/TextPlaceholderElement.cpp	2020-06-04 00:01:55 UTC (rev 262525)
@@ -43,8 +43,8 @@
     : HTMLDivElement { HTMLNames::divTag, document }
 {
     // FIXME: Move to User Agent stylesheet. See <https://webkit.org/b/208745>.
-    setInlineStyleProperty(CSSPropertyDisplay, CSSValueInlineBlock, false);
-    setInlineStyleProperty(CSSPropertyVerticalAlign, CSSValueBottom, false);
+    setInlineStyleProperty(CSSPropertyDisplay, size.width() ? CSSValueInlineBlock : CSSValueBlock, false);
+    setInlineStyleProperty(CSSPropertyVerticalAlign, CSSValueTop, false);
     setInlineStyleProperty(CSSPropertyVisibility, CSSValueHidden, true);
     setInlineStyleProperty(CSSPropertyWidth, size.width(), CSSUnitType::CSS_PX);
     setInlineStyleProperty(CSSPropertyHeight, size.height(), CSSUnitType::CSS_PX);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to