isContentEditable returns false for display:none contenteditable el…

…ements, but true for children https://bugs.webkit.org/show_bug.cgi?id=240834 Reviewed by Ryosuke Niwa. * Source/WebCore/dom/Node.cpp: (WebCore::computeEditabilityFromComputedStyle): Remove a `display: none` check. This check is wrong for two reasons: 1) It only consults the style of the immediate element, not whether or not that element is in a `display: none` subtree. This regressed in r160966. This causes a confusing situation where `display: none` affects the editability of only the element it is directly applied to. 2) isContentEditable is not specified to be affected by `display: none`. * LayoutTests/editing/editability/isContentEditable-in-display-none-expected.txt: Added. * LayoutTests/editing/editability/isContentEditable-in-display-none.html: Added. Add a test that dumps the editability of a variety of different configurations of contenteditable elements and their children. Before this change, this test would have said that #nonVisible (the display: none contenteditable element) was read-only, but its child <p> was editable. Now we agree with Gecko and Blink who both say that both are editable. * LayoutTests/fast/events/event-input-contentEditable-expected.txt: * LayoutTests/fast/events/event-input-contentEditable.html: Adjust this test to expect an input event to be dispatched for programmatic editing in display: none elements. Also add a sub-test that checks the same, but in a child element. Before this change, we would not dispatch an input event to the display: none contenteditable itself, but *would* dispatch an input event for input into a child element. Now we agree with Gecko, who dispatch events in both cases. Canonical link: https://commits.webkit.org/250921@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294753 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 24, 2022 · 927d4f76cff2e8c53450a594977f6ca912c8cae8 · 927d4f7
1 parent 43e8018
commit 927d4f76cff2e8c53450a594977f6ca912c8cae8
Showing 5 changed files with 86 additions and 7 deletions.
diff --git a/LayoutTests/editing/editability/isContentEditable-in-display-none-expected.txt b/LayoutTests/editing/editability/isContentEditable-in-display-none-expected.txt
@@ -0,0 +1,19 @@
+BODY: Read-only
+	#nonEditable: Read-only
+		Directly inside a normal div.
+		P: Read-only
+			In a child of a normal div.
+	#visible: Editable
+		Directly inside a contenteditable.
+		P: Editable
+			In a child of a contenteditable.
+	#nonVisible: Editable
+		Directly inside a display:none contenteditable.
+		P: Editable
+			In a child of a display:none contenteditable.
+	DIV: Read-only
+		#nonVisibleParent: Editable
+			Directly inside a contenteditable with a display:none parent.
+			P: Editable
+				In a child of a contenteditable with a display:none parent.
+
diff --git a/LayoutTests/editing/editability/isContentEditable-in-display-none.html b/LayoutTests/editing/editability/isContentEditable-in-display-none.html
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body { white-space: pre-wrap; }
+</style>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function visit(node, f, depth) {
+    f(node, depth);
+    for (let child of node.childNodes)
+        visit(child, f, depth + 1);
+}
+
+window.onload = function () {
+    let description = "";
+    visit(document.body, function (node, depth) {
+        let indent = "\t".repeat(depth);
+
+        if (node.nodeType == Node.TEXT_NODE) {
+            let trimmedContent = node.textContent.trim();
+            if (trimmedContent)
+                description += `${indent}${trimmedContent}\n`;
+            return;
+        }
+
+        let identifier = node.id ? `#${node.id}` : node.nodeName;
+        let editability = node.isContentEditable ? "Editable" : "Read-only";
+        description += `${indent}${identifier}: ${editability}\n`;
+    }, 0);
+    document.body.innerText = description;
+}
+</script>
+<body>
+<div id="nonEditable">Directly inside a normal div.
+    <p>In a child of a normal div.</p>
+</div>
+<div id="visible" contenteditable>Directly inside a contenteditable.
+    <p>In a child of a contenteditable.</p>
+</div>
+<div id="nonVisible" style="display: none" contenteditable>Directly inside a display:none contenteditable.
+    <p>In a child of a display:none contenteditable.</p>
+</div>
+<div style="display: none">
+<div id="nonVisibleParent" contenteditable>Directly inside a contenteditable with a display:none parent.
+    <p>In a child of a contenteditable with a display:none parent.</p>
+</div>
+</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/events/event-input-contentEditable-expected.txt b/LayoutTests/fast/events/event-input-contentEditable-expected.txt
@@ -11,6 +11,10 @@ PASS event.target.id is 'target3'
 PASS event.target.innerHTML is '<br>'
 PASS event.target.id is 'target4'
 PASS event.target.innerHTML is '<a href="http://www.example.com/">This text should be a link.</a>'
+PASS event.target.id is 'target5A'
+PASS event.target.innerHTML is 'Text'
+PASS event.target.id is 'target5B'
+PASS event.target.innerHTML is 'This <span id="target5BDestination">text</span> will not be rendered.'
 PASS event.target.id is 'target6parent'
 PASS event.target.innerHTML is '<a href="http://www.example.com/"><span id="target6start">Start,</span><span id="target6middle">Middle,</span><span id="target6end">End.</span></a>'
 PASS event.target.id is 'target7'

diff --git a/LayoutTests/fast/events/event-input-contentEditable.html b/LayoutTests/fast/events/event-input-contentEditable.html
@@ -67,13 +67,19 @@
 var target4 = setupForFiringTest('<p id="target4" contentEditable>This text should be a link.</p>', "<a href=\"http://www.example.com/\">This text should be a link.</a>");
 document.execCommand("createLink", false, "http://www.example.com/");
 
-// An event shouldn't be dispatched to a 'display:none' node.
-var target5 = makeTestTarget('<p id="target5" contentEditable>This will not be rendered.</p>');
-target5.addEventListener("input", function(evt) { testFailed("should not be reached"); });
-sel.selectAllChildren(target5);
-target5.style.display = "none";
+// An event should be dispatched to a 'display:none' node.
+var target5A = setupForFiringTest('<p id="target5A" contentEditable>This will not be rendered.</p>', 'Text');
+sel.selectAllChildren(target5A);
+target5A.style.display = "none";
 document.execCommand("insertText", false, "Text");
 
+// An event should be dispatched to a child of a 'display:none' node.
+var target5B = setupForFiringTest('<p id="target5B" contentEditable>This <span id="target5BDestination">replace</span> will not be rendered.</p>', 'This <span id="target5BDestination">text</span> will not be rendered.');
+var target5BDestination = document.getElementById("target5BDestination");
+sel.selectAllChildren(target5BDestination);
+target5B.style.display = "none";
+document.execCommand("insertText", false, "text");
+
 // The event should be dispatched from the editable root.
 makeTestTarget('<p id="target6parent" contentEditable><span id="target6start">Start,</span><span id="target6middle">Middle,</span><span id="target6end">End.</span></p>');
 var target6parent = document.getElementById("target6parent");

diff --git a/Source/WebCore/dom/Node.cpp b/Source/WebCore/dom/Node.cpp
@@ -764,8 +764,7 @@ static Node::Editability computeEditabilityFromComputedStyle(const Node& startNo
         auto* style = node->isDocumentNode() ? node->renderStyle() : const_cast<Node*>(node)->computedStyle();
         if (!style)
             continue;
-        if (style->display() == DisplayType::None)
-            continue;
+
         // Elements with user-select: all style are considered atomic
         // therefore non editable.
         if (treatment == Node::UserSelectAllIsAlwaysNonEditable && style->effectiveUserSelect() == UserSelect::All)