Merge pull request #5070 from tsiegleauq/strange-pdf-line-number-issue

Fix format motions hidden change discrepancy

client/src/app/core/repositories/motions/motion-repository.service.ts

@@ -600,43 +600,37 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
                         highlightLine
                     );
                 case ChangeRecoMode.Diff:
-                    let text = '';
+                    const text = [];
-                    const changesToShow = changes.filter(change => {
+                    const changesToShow = changes.filter(change => change.showInDiffView());
-                        return change.showInDiffView();
+
-                    });
+                    for (let i = 0; i < changesToShow.length; i++) {
-                    changesToShow.forEach((change: ViewUnifiedChange, idx: number) => {
+                        text.push(
-                        if (idx === 0) {
+                            this.diff.extractMotionLineRange(
-                            text += this.diff.extractMotionLineRange(
                                 targetMotion.text,
                                 {
-                                    from: 1,
+                                    from: i === 0 ? 1 : changesToShow[i - 1].getLineTo(),
-                                    to: change.getLineFrom()
+                                    to: changesToShow[i].getLineFrom()
                                 },
                                 true,
                                 lineLength,
                                 highlightLine
+                            )
                         );
-                        } else if (changes[idx - 1].getLineTo() < change.getLineFrom()) {
+
-                            text += this.diff.extractMotionLineRange(
+                        text.push(
-                                targetMotion.text,
+                            this.diff.getChangeDiff(targetMotion.text, changesToShow[i], lineLength, highlightLine)
-                                {
-                                    from: changes[idx - 1].getLineTo(),
-                                    to: change.getLineFrom()
-                                },
-                                true,
-                                lineLength,
-                                highlightLine
                         );
                     }
-                        text += this.diff.getChangeDiff(targetMotion.text, change, lineLength, highlightLine);
+
-                    });
+                    text.push(
-                    text += this.diff.getTextRemainderAfterLastChange(
+                        this.diff.getTextRemainderAfterLastChange(
                             targetMotion.text,
                             changesToShow,
                             lineLength,
                             highlightLine
+                        )
                     );
-                    return text;
+                    return text.join('');
                 case ChangeRecoMode.Final:
                     const appliedChanges: ViewUnifiedChange[] = changes.filter(change => change.showInFinalView());
                     return this.diff.getTextWithChanges(targetMotion.text, appliedChanges, lineLength, highlightLine);

client/src/app/core/ui-services/diff.service.ts

@@ -1333,7 +1333,6 @@ export class DiffService {
         }
 
         const fragment = this.htmlToFragment(htmlIn);
-
         this.insertInternalLineMarkers(fragment);
         if (toLine === null) {
             const internalLineMarkers = fragment.querySelectorAll('OS-LINEBREAK'),
@@ -1363,6 +1362,7 @@ export class DiffService {
         fromChildTraceAbs.shift();
         const previousHtml = this.serializePartialDomToChild(fragment, fromChildTraceAbs, false);
         toChildTraceAbs.shift();
+
         const followingHtml = this.serializePartialDomFromChild(fragment, toChildTraceAbs, false);
 
         let currNode: Node = fromLineNode,
@@ -1552,54 +1552,38 @@ export class DiffService {
      * @returns {Node[]}
      */
     public replaceLinesMergeNodeArrays(nodes1: Node[], nodes2: Node[]): Node[] {
-        if (nodes1.length === 0) {
+        if (nodes1.length === 0 || nodes2.length === 0) {
-            return nodes2;
+            return nodes1.length ? nodes1 : nodes2;
-        }
-        if (nodes2.length === 0) {
-            return nodes1;
         }
 
-        const out = [];
+        const out: Node[] = nodes1.slice(0, -1);
-        for (let i = 0; i < nodes1.length - 1; i++) {
+        const lastNode: Node = nodes1[nodes1.length - 1];
-            out.push(nodes1[i]);
+        const firstNode: Node = nodes2[0];
-        }
-
-        const lastNode = nodes1[nodes1.length - 1],
-            firstNode = nodes2[0];
         if (lastNode.nodeType === TEXT_NODE && firstNode.nodeType === TEXT_NODE) {
-            const newTextNode = lastNode.ownerDocument.createTextNode(lastNode.nodeValue + firstNode.nodeValue);
+            const newTextNode: Text = lastNode.ownerDocument.createTextNode(lastNode.nodeValue + firstNode.nodeValue);
             out.push(newTextNode);
         } else if (lastNode.nodeName === firstNode.nodeName) {
-            const lastElement = <Element>lastNode,
+            const lastElement: Element = lastNode as Element;
-                newNode = lastNode.ownerDocument.createElement(lastNode.nodeName);
+            const newNode: HTMLElement = lastNode.ownerDocument.createElement(lastNode.nodeName);
-            for (let i = 0; i < lastElement.attributes.length; i++) {
+
-                const attr = lastElement.attributes[i];
+            for (const attr of Array.from(lastElement.attributes)) {
                 newNode.setAttribute(attr.name, attr.value);
             }
 
             // Remove #text nodes inside of List elements (OL/UL), as they are confusing
-            let lastChildren, firstChildren;
+            let lastChildren: Node[];
+            let firstChildren: Node[];
             if (lastElement.nodeName === 'OL' || lastElement.nodeName === 'UL') {
-                lastChildren = [];
+                lastChildren = Array.from(lastElement.childNodes).filter(child => child.nodeType === ELEMENT_NODE);
-                firstChildren = [];
+                firstChildren = Array.from(firstNode.childNodes).filter(child => child.nodeType === ELEMENT_NODE);
-                for (let i = 0; i < firstNode.childNodes.length; i++) {
-                    if (firstNode.childNodes[i].nodeType === ELEMENT_NODE) {
-                        firstChildren.push(firstNode.childNodes[i]);
-                    }
-                }
-                for (let i = 0; i < lastElement.childNodes.length; i++) {
-                    if (lastElement.childNodes[i].nodeType === ELEMENT_NODE) {
-                        lastChildren.push(lastElement.childNodes[i]);
-                    }
-                }
             } else {
-                lastChildren = lastElement.childNodes;
+                lastChildren = Array.from(lastElement.childNodes);
-                firstChildren = firstNode.childNodes;
+                firstChildren = Array.from(firstNode.childNodes);
             }
 
-            const children = this.replaceLinesMergeNodeArrays(lastChildren, firstChildren);
+            const children = this.replaceLinesMergeNodeArrays(lastChildren, firstChildren) as Node[];
-            for (let i = 0; i < children.length; i++) {
+            for (const child of children) {
-                newNode.appendChild(children[i]);
+                newNode.appendChild(child);
             }
 
             out.push(newNode);
@@ -1612,11 +1596,7 @@ export class DiffService {
             }
         }
 
-        for (let i = 1; i < nodes2.length; i++) {
+        return out.concat(nodes2.slice(1, nodes2.length));
-            out.push(nodes2[i]);
-        }
-
-        return out;
     }
 
     /**
@@ -1755,7 +1735,8 @@ export class DiffService {
             this.removeCSSClass(forgottenSplitClasses[i], 'os-split-after');
         }
 
-        return this.serializeDom(mergedFragment, true);
+        const replacedHtml = this.serializeDom(mergedFragment, true);
+        return replacedHtml;
     }
 
     /**

client/src/app/site/motions/modules/motion-detail/components/motion-detail/motion-detail.component.ts

@@ -854,7 +854,14 @@ export class MotionDetailComponent extends BaseViewComponent implements OnInit,
     public getFormattedTextPlain(): string {
         // Prevent this.allChangingObjects to be reordered from within formatMotion
         const changes: ViewUnifiedChange[] = Object.assign([], this.getAllTextChangingObjects());
-        return this.repo.formatMotion(this.motion.id, this.crMode, changes, this.lineLength, this.highlightedLine);
+        const formatedText = this.repo.formatMotion(
+            this.motion.id,
+            this.crMode,
+            changes,
+            this.lineLength,
+            this.highlightedLine
+        );
+        return formatedText;
     }
 
     /**