diff --git a/ChangeLog.md b/ChangeLog.md index 6f37a4088..179b0f281 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,3 +1,12 @@ +# Changes between 0.5.6 and 0.5.7 + +## WebODF + +### Fixes + +* Fix breaking all empty annotations on merging the paragraph they are contained in with the one before ([#877](https://github.com/kogmbh/WebODF/pull/877))) + + # Changes between 0.5.5 and 0.5.6 ## WebODF diff --git a/webodf/lib/core/DomUtils.js b/webodf/lib/core/DomUtils.js index a8d77389a..23a7d37b9 100644 --- a/webodf/lib/core/DomUtils.js +++ b/webodf/lib/core/DomUtils.js @@ -579,20 +579,29 @@ /** * Removes all unwanted nodes from targetNode includes itself. + * The nodeFilter defines which nodes should be removed (NodeFilter.FILTER_REJECT), + * should be skipped including the subtree (NodeFilter.FILTER_SKIP) or should be kept + * and their subtree checked further (NodeFilter.FILTER_ACCEPT). * @param {!Node} targetNode - * @param {function(!Node):!boolean} shouldRemove check whether a node should be removed or not + * @param {!function(!Node) : !number} nodeFilter * @return {?Node} parent of targetNode */ - function removeUnwantedNodes(targetNode, shouldRemove) { + function removeUnwantedNodes(targetNode, nodeFilter) { var parent = targetNode.parentNode, node = targetNode.firstChild, + filterResult = nodeFilter(targetNode), next; + + if (filterResult === NodeFilter.FILTER_SKIP) { + return parent; + } + while (node) { next = node.nextSibling; - removeUnwantedNodes(node, shouldRemove); + removeUnwantedNodes(node, nodeFilter); node = next; } - if (parent && shouldRemove(targetNode)) { + if (parent && (filterResult === NodeFilter.FILTER_REJECT)) { mergeIntoParent(targetNode); } return parent; diff --git a/webodf/lib/odf/CollapsingRules.js b/webodf/lib/odf/CollapsingRules.js index 18f5450b0..765fea603 100644 --- a/webodf/lib/odf/CollapsingRules.js +++ b/webodf/lib/odf/CollapsingRules.js @@ -22,7 +22,7 @@ * @source: https://github.com/kogmbh/WebODF/ */ -/*global odf, core, Node*/ +/*global odf, core, Node, NodeFilter*/ /** * Defines a set of rules for how elements can be collapsed based on whether they contain ODT content (e.g., @@ -36,14 +36,15 @@ odf.CollapsingRules = function CollapsingRules(rootNode) { domUtils = core.DomUtils; /** - * Returns true if a given node is odf node or a text node that has a odf parent. + * Returns NodeFilter value if a given node is odf node or a text node that has a odf parent. * @param {!Node} node - * @return {!boolean} + * @return {!number} */ - function shouldRemove(node) { - return odfUtils.isODFNode(node) + function filterOdfNodesToRemove(node) { + var isToRemove = odfUtils.isODFNode(node) || (node.localName === "br" && odfUtils.isLineBreak(node.parentNode)) || (node.nodeType === Node.TEXT_NODE && odfUtils.isODFNode(/** @type {!Node}*/(node.parentNode))); + return isToRemove ? NodeFilter.FILTER_REJECT : NodeFilter.FILTER_ACCEPT; } /** @@ -69,7 +70,7 @@ odf.CollapsingRules = function CollapsingRules(rootNode) { parent.removeChild(targetNode); } else { // removes all odf nodes - parent = domUtils.removeUnwantedNodes(targetNode, shouldRemove); + parent = domUtils.removeUnwantedNodes(targetNode, filterOdfNodesToRemove); } if (parent && isCollapsibleContainer(parent)) { return mergeChildrenIntoParent(parent); diff --git a/webodf/lib/ops/OpMergeParagraph.js b/webodf/lib/ops/OpMergeParagraph.js index 7b723696e..90fb2468b 100644 --- a/webodf/lib/ops/OpMergeParagraph.js +++ b/webodf/lib/ops/OpMergeParagraph.js @@ -22,7 +22,7 @@ * @source: https://github.com/kogmbh/WebODF/ */ -/*global ops, runtime, odf, core, Node*/ +/*global ops, runtime, odf, core, Node, NodeFilter*/ /** * Merges two adjacent paragraphs together into the first paragraph. The destination paragraph @@ -68,10 +68,13 @@ ops.OpMergeParagraph = function OpMergeParagraph() { /** * Returns true if the supplied node is an ODF grouping element with no content * @param {!Node} element - * @return {!boolean} + * @return {!number} */ - function isEmptyGroupingElement(element) { - return odfUtils.isGroupingElement(element) && odfUtils.hasNoODFContent(element); + function filterEmptyGroupingElementToRemove(element) { + if (odf.OdfUtils.isInlineRoot(element)) { + return NodeFilter.FILTER_SKIP; + } + return odfUtils.isGroupingElement(element) && odfUtils.hasNoODFContent(element) ? NodeFilter.FILTER_REJECT : NodeFilter.FILTER_ACCEPT; } /** @@ -93,7 +96,7 @@ ops.OpMergeParagraph = function OpMergeParagraph() { // Empty spans need to be cleaned up on merge, as remove text only removes things that contain text content // Child is moved across before collapsing so any foreign sub-elements are collapsed up the chain next to // the destination location - domUtils.removeUnwantedNodes(child, isEmptyGroupingElement); + domUtils.removeUnwantedNodes(child, filterEmptyGroupingElementToRemove); } child = source.firstChild; } diff --git a/webodf/tests/core/DomUtilsTests.js b/webodf/tests/core/DomUtilsTests.js index cf80e25b8..438df77a2 100644 --- a/webodf/tests/core/DomUtilsTests.js +++ b/webodf/tests/core/DomUtilsTests.js @@ -515,7 +515,7 @@ core.DomUtilsTests = function DomUtilsTests(runner) { p.appendChild(span3); t.doc.appendChild(p); t.parent = t.utils.removeUnwantedNodes(p, function (node) { - return node !== null; + return (node !== null) ? NodeFilter.FILTER_REJECT : NodeFilter.FILTER_ACCEPT; }); r.shouldBe(t, "t.parent", "t.doc"); r.shouldBe(t, "t.parent.childNodes.length", "0"); @@ -536,7 +536,7 @@ core.DomUtilsTests = function DomUtilsTests(runner) { p.appendChild(span3); t.doc.appendChild(p); t.parent = t.utils.removeUnwantedNodes(p, function (node) { - return node.localName === 'span'; + return node.localName === 'span' ? NodeFilter.FILTER_REJECT : NodeFilter.FILTER_ACCEPT; }); r.shouldBe(t, "t.parent", "t.doc"); r.shouldBe(t, "t.parent.firstChild.localName", "'p'"); @@ -546,6 +546,32 @@ core.DomUtilsTests = function DomUtilsTests(runner) { r.shouldBe(t, "t.parent.firstChild.childNodes[2].firstChild.textContent", "'test'"); } + function removeUnwantedNodes_SkipDiv() { + var p = document.createElement("p"), + span1 = document.createElement("span"), + div = document.createElement("div"), + divspan = document.createElement("span"), + span3 = document.createElement("span"), + b = document.createElement("b"); + b.textContent = "test"; + span1.textContent = "hello"; + divspan.textContent = "world"; + span3.appendChild(b); + p.appendChild(span1); + p.appendChild(div); + div.appendChild(divspan); + p.appendChild(span3); + t.doc.appendChild(p); + t.parent = t.utils.removeUnwantedNodes(p, function (node) { + return (node.localName === "div") ? NodeFilter.FILTER_SKIP : NodeFilter.FILTER_REJECT; + }); + r.shouldBe(t, "t.parent", "t.doc"); + r.shouldBe(t, "t.parent.childNodes.length", "1"); + r.shouldBe(t, "t.parent.firstChild.localName", "'div'"); + r.shouldBe(t, "t.parent.firstChild.childNodes.length", "1"); + r.shouldBe(t, "t.parent.firstChild.firstChild.localName", "'span'"); + } + function removeAllChildNodes_None() { var p = document.createElement("p"); t.doc.appendChild(p); @@ -818,6 +844,7 @@ core.DomUtilsTests = function DomUtilsTests(runner) { removeUnwantedNodes_DiscardAll, removeUnwantedNodes_DiscardSpanOnly, + removeUnwantedNodes_SkipDiv, removeAllChildNodes_None, removeAllChildNodes_ElementAndTextNodes, diff --git a/webodf/tests/ops/operationtests.xml b/webodf/tests/ops/operationtests.xml index bbe901e4b..12ec7d91a 100644 --- a/webodf/tests/ops/operationtests.xml +++ b/webodf/tests/ops/operationtests.xml @@ -528,6 +528,29 @@ abcdefgh + + abc + Alice + 2013-08-05T12:34:07.061Z + + + + + + d + + + + abc + Alice + 2013-08-05T12:34:07.061Z + + + + + + d + abcdefgh