Title: [294753] trunk
- Revision
- 294753
- Author
- timothy_hor...@apple.com
- Date
- 2022-05-24 10:28:58 -0700 (Tue, 24 May 2022)
Log Message
isContentEditable returns false for `display:none` contenteditable elements, 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
Modified Paths
Added Paths
Diff
Added: trunk/LayoutTests/editing/editability/isContentEditable-in-display-none-expected.txt (0 => 294753)
--- trunk/LayoutTests/editing/editability/isContentEditable-in-display-none-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/editability/isContentEditable-in-display-none-expected.txt 2022-05-24 17:28:58 UTC (rev 294753)
@@ -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.
+
Added: trunk/LayoutTests/editing/editability/isContentEditable-in-display-none.html (0 => 294753)
--- trunk/LayoutTests/editing/editability/isContentEditable-in-display-none.html (rev 0)
+++ trunk/LayoutTests/editing/editability/isContentEditable-in-display-none.html 2022-05-24 17:28:58 UTC (rev 294753)
@@ -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>
\ No newline at end of file
Modified: trunk/LayoutTests/fast/events/event-input-contentEditable-expected.txt (294752 => 294753)
--- trunk/LayoutTests/fast/events/event-input-contentEditable-expected.txt 2022-05-24 16:57:53 UTC (rev 294752)
+++ trunk/LayoutTests/fast/events/event-input-contentEditable-expected.txt 2022-05-24 17:28:58 UTC (rev 294753)
@@ -11,6 +11,10 @@
PASS event.target.innerHTML is '<br>'
PASS event.target.id is 'target4'
PASS event.target.innerHTML is '<a href="" 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="" id="target6start">Start,</span><span id="target6middle">Middle,</span><span id="target6end">End.</span></a>'
PASS event.target.id is 'target7'
Modified: trunk/LayoutTests/fast/events/event-input-contentEditable.html (294752 => 294753)
--- trunk/LayoutTests/fast/events/event-input-contentEditable.html 2022-05-24 16:57:53 UTC (rev 294752)
+++ trunk/LayoutTests/fast/events/event-input-contentEditable.html 2022-05-24 17:28:58 UTC (rev 294753)
@@ -67,13 +67,19 @@
var target4 = setupForFiringTest('<p id="target4" contentEditable>This text should be a link.</p>', "<a href="" 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");
Modified: trunk/Source/WebCore/dom/Node.cpp (294752 => 294753)
--- trunk/Source/WebCore/dom/Node.cpp 2022-05-24 16:57:53 UTC (rev 294752)
+++ trunk/Source/WebCore/dom/Node.cpp 2022-05-24 17:28:58 UTC (rev 294753)
@@ -764,8 +764,7 @@
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)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
