Changeset 294753 in webkit

Timestamp:

May 24, 2022, 10:28:58 AM (4 years ago)

Author:

timothy_horton@apple.com

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

Location:

trunk

Files:

• LayoutTests/editing/editability/isContentEditable-in-display-none-expected.txt (added)
• LayoutTests/editing/editability/isContentEditable-in-display-none.html (added)
• LayoutTests/fast/events/event-input-contentEditable-expected.txt (modified) (1 diff)
• LayoutTests/fast/events/event-input-contentEditable.html (modified) (1 diff)
• Source/WebCore/dom/Node.cpp (modified) (1 diff)

trunk/LayoutTests/fast/events/event-input-contentEditable-expected.txt

@@ -12 +12 @@
 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>'

trunk/LayoutTests/fast/events/event-input-contentEditable.html

@@ -68 +68 @@
 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.

trunk/Source/WebCore/dom/Node.cpp

@@ -765 +765 @@
         if (!style)
             continue;
-        if (style->display() == DisplayType::None)
-            continue;
+
         // Elements with user-select: all style are considered atomic
         // therefore non editable.