Should type in contenteditable div even if it has invisible child with contenteditable=false (#2205)

src/client/automation/playback/type/type-text.js

@@ -49,13 +49,15 @@ function _updateSelectionAfterDeletionContent (element, selection) {
     return selection;
 }
 
-function _typeTextInElementNode (elementNode, text) {
+function _typeTextInElementNode (elementNode, text, offset) {
     var nodeForTyping  = document.createTextNode(text);
     var textLength     = text.length;
     var selectPosition = { node: nodeForTyping, offset: textLength };
 
     if (domUtils.getTagName(elementNode) === 'br')
         elementNode.parentNode.insertBefore(nodeForTyping, elementNode);
+    else if (offset > 0)
+        elementNode.insertBefore(nodeForTyping, elementNode.childNodes[offset]);
     else
         elementNode.appendChild(nodeForTyping);
 
@@ -126,7 +128,7 @@ function _typeTextToContentEditable (element, text) {
 
     // NOTE: we can type only to the text nodes; for nodes with the 'element-node' type, we use a special behavior
     if (domUtils.isElementNode(startNode)) {
-        _typeTextInElementNode(startNode, text);
+        _typeTextInElementNode(startNode, text, currentSelection.startPos.offset);
 
         afterContentChanged();
         return;

src/client/core/utils/content-editable.js

@@ -1,6 +1,6 @@
 import * as domUtils from './dom';
 import * as arrayUtils from './array';
-
+import * as styleUtils from './style';
 
 //nodes utils
 function getOwnFirstVisibleTextNode (el) {
@@ -34,12 +34,16 @@ function getOwnPreviousVisibleSibling (el) {
     return sibling;
 }
 
-function hasChildren (node) {
-    return node.childNodes && domUtils.getChildNodesLength(node.childNodes);
+function getVisibleChildren (node) {
+    return arrayUtils.filter(node.childNodes, child => domUtils.isTextNode(child) || domUtils.isElementNode(child) && styleUtils.isElementVisible(child));
+}
+
+function hasVisibleChildren (node) {
+    return getVisibleChildren(node).length;
 }
 
-function isElementWithChildren (node) {
-    return domUtils.isElementNode(node) || hasChildren(node);
+function hasSelectableChildren (node) {
+    return arrayUtils.some(node.childNodes, child => isNodeSelectable(child, true));
 }
 
 //NOTE: before such elements (like div or p) adds line breaks before and after it
@@ -118,7 +122,7 @@ export function getFirstVisibleTextNode (el) {
 
         if (isVisibleTextNode(curNode))
             return curNode;
-        else if (domUtils.isRenderedNode(curNode) && isElementWithChildren(curNode) && !isNotContentEditableElement) {
+        else if (domUtils.isRenderedNode(curNode) && hasVisibleChildren(curNode) && !isNotContentEditableElement) {
             child = getFirstVisibleTextNode(curNode);
 
             if (child)
@@ -148,7 +152,7 @@ export function getLastTextNode (el, onlyVisible) {
 
         if (visibleTextNode)
             return curNode;
-        else if (domUtils.isRenderedNode(curNode) && isElementWithChildren(curNode) && !isNotContentEditableElement) {
+        else if (domUtils.isRenderedNode(curNode) && hasVisibleChildren(curNode) && !isNotContentEditableElement) {
             child = getLastTextNode(curNode, false);
 
             if (child)
@@ -293,8 +297,10 @@ function getSelectedPositionInParentByOffset (node, offset) {
 
     // NOTE: we try to find text node
     while (!isSkippableNode(currentNode) && domUtils.isElementNode(currentNode)) {
-        if (hasChildren(currentNode))
-            currentNode = currentNode.childNodes[isSearchForLastChild ? currentNode.childNodes.length - 1 : 0];
+        const visibleChildren = getVisibleChildren(currentNode);
+
+        if (visibleChildren.length)
+            currentNode = visibleChildren[isSearchForLastChild ? visibleChildren.length - 1 : 0];
         else {
             //NOTE: if we didn't find a text node then always set offset to zero
             currentOffset = 0;
@@ -321,7 +327,7 @@ function getSelectionStart (el, selection, inverseSelection) {
     };
 
     //NOTE: window.getSelection() can't returns not rendered node like selected node, so we shouldn't check it
-    if ((domUtils.isTheSameNode(el, startNode) || domUtils.isElementNode(startNode)) && hasChildren(startNode))
+    if ((domUtils.isTheSameNode(el, startNode) || domUtils.isElementNode(startNode)) && hasSelectableChildren(startNode))
         correctedStartPosition = getSelectedPositionInParentByOffset(startNode, startOffset);
 
     return {
@@ -340,7 +346,7 @@ function getSelectionEnd (el, selection, inverseSelection) {
     };
 
     //NOTE: window.getSelection() can't returns not rendered node like selected node, so we shouldn't check it
-    if ((domUtils.isTheSameNode(el, endNode) || domUtils.isElementNode(endNode)) && hasChildren(endNode))
+    if ((domUtils.isTheSameNode(el, endNode) || domUtils.isElementNode(endNode)) && hasSelectableChildren(endNode))
         correctedEndPosition = getSelectedPositionInParentByOffset(endNode, endOffset);
 
     return {
@@ -368,6 +374,32 @@ export function getSelectionEndPosition (el, selection, inverseSelection) {
     return calculatePositionByNodeAndOffset(el, correctedSelectionEnd);
 }
 
+function getElementOffset (target) {
+    let offset = 0;
+
+    const firstBreakElement = arrayUtils.find(target.childNodes, (node, index) => {
+        offset = index;
+        return domUtils.getTagName(node) === 'br';
+    });
+
+    return firstBreakElement ? offset : 0;
+}
+
+function isNodeSelectable (node, includeDescendants = false) {
+    if (domUtils.isTextNode(node))
+        return true;
+
+    if (!domUtils.isElementNode(node))
+        return false;
+
+    const visibleChildren       = getVisibleChildren(node);
+    const isContentEditableRoot = !domUtils.isContentEditableElement(node.parentNode);
+    const selectableChildren    = arrayUtils.filter(visibleChildren, child => isNodeSelectable(child));
+    const breakLineElements     = arrayUtils.filter(visibleChildren, child => domUtils.getTagName(child) === 'br');
+
+    return selectableChildren.length ? includeDescendants : isContentEditableRoot || breakLineElements.length;
+}
+
 export function calculateNodeAndOffsetByPosition (el, offset) {
     var point = {
         node:   null,
@@ -397,13 +429,17 @@ export function calculateNodeAndOffsetByPosition (el, offset) {
             }
         }
         else if (domUtils.isElementNode(target)) {
-            if (point.offset === 0 && !getContentEditableValue(target).length) {
-                point.node = target;
+            if (!styleUtils.isElementVisible(target))
+                return point;
+
+            if (point.offset === 0 && isNodeSelectable(target)) {
+                point.node   = target;
+                point.offset = getElementOffset(target);
                 return point;
             }
             if (!point.node && (isNodeBlockWithBreakLine(el, target) || isNodeAfterNodeBlockWithBreakLine(el, target)))
                 point.offset--;
-            else if (!childNodesLength && domUtils.isElementNode(target) && domUtils.getTagName(target) === 'br')
+            else if (!childNodesLength && domUtils.getTagName(target) === 'br')
                 point.offset--;
         }
 
@@ -499,23 +535,22 @@ export function getLastVisiblePosition (el) {
     return 0;
 }
 
-//contents util
-export function getContentEditableValue (target) {
-    var elementValue     = '';
+function getContentEditableNodes (target) {
+    var result           = [];
     var childNodes       = target.childNodes;
     var childNodesLength = domUtils.getChildNodesLength(childNodes);
 
-    if (isSkippableNode(target))
-        return elementValue;
-
-    if (!childNodesLength && domUtils.isTextNode(target))
-        return target.nodeValue;
-    else if (childNodesLength === 1 && domUtils.isTextNode(childNodes[0]))
-        return childNodes[0].nodeValue;
+    if (!isSkippableNode(target) && !childNodesLength && domUtils.isTextNode(target))
+        result.push(target);
 
     arrayUtils.forEach(childNodes, node => {
-        elementValue += getContentEditableValue(node);
+        result = result.concat(getContentEditableNodes(node));
     });
 
-    return elementValue;
+    return result;
+}
+
+// contents util
+export function getContentEditableValue (target) {
+    return arrayUtils.map(getContentEditableNodes(target), node => node.nodeValue).join('');
 }

src/client/core/utils/style.js

@@ -12,6 +12,7 @@ export var getElementPadding    = hammerhead.utils.style.getElementPadding;
 export var getElementScroll     = hammerhead.utils.style.getElementScroll;
 export var getOptionHeight      = hammerhead.utils.style.getOptionHeight;
 export var getSelectElementSize = hammerhead.utils.style.getSelectElementSize;
+export var isElementVisible     = hammerhead.utils.style.isElementVisible;
 export var isSelectVisibleChild = hammerhead.utils.style.isVisibleChild;
 export var getWidth             = hammerhead.utils.style.getWidth;
 export var getHeight            = hammerhead.utils.style.getHeight;
@@ -40,10 +41,14 @@ var getAncestorsAndSelf = function (node) {
     return getAncestors(node).concat([node]);
 };
 
-var isVisibilityHiddenTextNode = function (textNode) {
-    var el = domUtils.isTextNode(textNode) ? textNode.parentNode : null;
+var isVisibilityHiddenNode = function (node) {
+    var ancestors = getAncestorsAndSelf(node);
+
+    return some(ancestors, ancestor => domUtils.isElementNode(ancestor) && get(ancestor, 'visibility') === 'hidden');
+};
 
-    return el && get(el, 'visibility') === 'hidden';
+var isVisibilityHiddenTextNode = function (textNode) {
+    return domUtils.isTextNode(textNode) && isVisibilityHiddenNode(textNode);
 };
 
 var isHiddenNode = function (node) {
@@ -135,22 +140,6 @@ export function hasDimensions (el) {
     return el && !(el.offsetHeight <= 0 && el.offsetWidth <= 0);
 }
 
-export function isElementHidden (el) {
-    //NOTE: it's like jquery ':hidden' selector
-    if (get(el, 'display') === 'none' || !hasDimensions(el) || el.type && el.type === 'hidden')
-        return true;
-
-    var elements       = domUtils.findDocument(el).querySelectorAll('*');
-    var hiddenElements = [];
-
-    for (var i = 0; i < elements.length; i++) {
-        if (get(elements[i], 'display') === 'none' || !hasDimensions(elements[i]))
-            hiddenElements.push(elements[i]);
-    }
-
-    return domUtils.containsElement(hiddenElements, el);
-}
-
 export function set (el, style, value) {
     if (typeof style === 'string')
         styleUtils.set(el, style, value);

src/client/core/utils/text-selection.js

@@ -349,8 +349,14 @@ export function selectByNodesAndOffsets (startPos, endPos, needFocus) {
 
     var startNodeLength = startNode.nodeValue ? startNode.length : 0;
     var endNodeLength   = endNode.nodeValue ? endNode.length : 0;
-    var startOffset     = Math.min(startNodeLength, startPos.offset);
-    var endOffset       = Math.min(endNodeLength, endPos.offset);
+    var startOffset     = startPos.offset;
+    var endOffset       = endPos.offset;
+
+    if (!domUtils.isElementNode(startNode) || !startOffset)
+        startOffset = Math.min(startNodeLength, startPos.offset);
+
+    if (!domUtils.isElementNode(endNode) || !endOffset)
+        endOffset = Math.min(endNodeLength, endPos.offset);
 
     var parentElement = contentEditable.findContentEditableParent(startNode);
     var inverse       = isInverseSelectionContentEditable(parentElement, startPos, endPos);

test/functional/fixtures/regression/gh-2205/pages/index.html

@@ -0,0 +1,101 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <title>Title</title>
+    <style>
+        #editor1.focused .placeholder {
+            display: none;
+        }
+
+        #editor2.focused .placeholder {
+            visibility: hidden;
+        }
+
+        #editor3 .placeholder {
+            display: none;
+        }
+
+        .placeholder {
+            color: #aaa;
+            font-style: italic;
+            height: 0;
+            pointer-events: none;
+        }
+
+        .editor {
+            padding: 4px 8px 4px 14px;
+            line-height: 1.2;
+            outline: none;
+        }
+
+        .editor {
+            word-wrap: break-word;
+            white-space: pre-wrap;
+            font-variant-ligatures: none;
+        }
+
+        .editor {
+            position: relative;
+        }
+
+        .outer-editor {
+            background: white;
+            color: black;
+            background-clip: padding-box;
+            border-radius: 4px;
+            border: 2px solid rgba(0, 0, 0, 0.2);
+            padding: 5px 0;
+            margin-bottom: 23px;
+        }
+
+        .outer-editor {
+            border: 1px solid grey;
+        }
+
+        .editor p:first-child {
+            margin-top: 10px;
+        }
+
+        .editor p {
+            margin-bottom: 1em;
+        }
+    </style>
+</head>
+<body>
+
+<h2>Display: none</h2>
+<div class="outer-editor">
+    <div id="editor1" contenteditable="true" class="editor"
+         onmousedown="event.currentTarget.className += ' focused'"></div>
+</div>
+
+<h2>Visibility: hidden</h2>
+<div class="outer-editor">
+    <div id="editor2" contenteditable="true" class="editor"
+         onmousedown="event.currentTarget.className += ' focused'"></div>
+</div>
+
+<h2>Two hidden divs inside</h2>
+<div class="outer-editor">
+    <div id="editor3" contenteditable="true" class="editor"><div><div class="placeholder"></div><div class="placeholder">test</div></div></div>
+</div>
+
+<div id="placeholder1" contenteditable="false" class="placeholder">Type here...</div>
+<div id="placeholder2" contenteditable="false" class="placeholder">Type here...</div>
+
+<script type="text/javascript">
+    function createEditor (editor, placeholder) {
+        var paragraph = document.createElement('p');
+        var lineBreak = document.createElement('br');
+
+        editor.appendChild(paragraph);
+        paragraph.appendChild(placeholder);
+        paragraph.appendChild(lineBreak);
+    }
+
+    createEditor(document.getElementById('editor1'), document.getElementById('placeholder1'));
+    createEditor(document.getElementById('editor2'), document.getElementById('placeholder2'));
+</script>
+</body>
+</html>

test/functional/fixtures/regression/gh-2205/test.js

@@ -0,0 +1,5 @@
+describe('[Regression](GH-2205)', function () {
+    it('Should type in div if it has an invisible child with contententeditable=false', function () {
+        return runTests('testcafe-fixtures/index.js');
+    });
+});

test/functional/fixtures/regression/gh-2205/testcafe-fixtures/index.js

@@ -0,0 +1,27 @@
+import { Selector } from 'testcafe';
+
+fixture `GH-2205 - Should type in div if it has an invisible child with contententeditable=false`
+    .page `http://localhost:3000/fixtures/regression/gh-2205/pages/index.html`;
+
+async function typeAndCheck (t, editorId) {
+    const editor = Selector(editorId);
+
+    await t
+        .click(editor)
+        .typeText(editor, 'Hello')
+        .wait(1000)
+        .expect(editor.innerText).contains('Hello');
+}
+
+test(`Click on div with display:none placeholder`, async t => {
+    await typeAndCheck(t, '#editor1');
+});
+
+test(`Click on div with visibility:hidden placeholder`, async t => {
+    await typeAndCheck(t, '#editor2');
+});
+
+test(`Click on div with two invisible placeholders`, async t => {
+    await typeAndCheck(t, '#editor3');
+});
+