From f3e5b077f67606dbf854f7e8a5d34ba1623e9aa5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Rogowicz?= <rogowicz.pawel@gmail.com>
Date: Mon, 11 Dec 2017 12:07:23 +0100
Subject: [PATCH] [BUGFIX] Use stateIdentifier instead of Uid for SVG tree
 nodes

Change Uid to stateIdentifier because one Uid can occur in the pagetree
many times, for example in DB mounts.
We need to know what node is visible or what node was clicked or what
node was mouse over / mouse leave, e.t.c.

This change also fixes:
- fix MP title position and visibility
- fix changing node name when the same node is multiple in page tree
(pagetree, mount points)
- modified the acceptance tests

Releases: master
Resolves: #83280
Change-Id: I57716db1f2850f0c4b1432651ed391ece5921fec
Reviewed-on: https://review.typo3.org/55020
Tested-by: TYPO3com <no-reply@typo3.com>
Tested-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
Reviewed-by: Tymoteusz Motylewski <t.motylewski@gmail.com>
Tested-by: Tymoteusz Motylewski <t.motylewski@gmail.com>
Reviewed-by: Alexander Opitz <opitz.alexander@googlemail.com>
Tested-by: Alexander Opitz <opitz.alexander@googlemail.com>
---
 .../Public/JavaScript/PageTree/PageTree.js    | 10 ++--
 .../JavaScript/PageTree/PageTreeDragDrop.js   |  8 +--
 .../Resources/Public/JavaScript/SvgTree.js    | 53 ++++++++++++-------
 .../Backend/Formhandler/CategoryTreeCest.php  |  6 +--
 .../Backend/Page/AddPageInPageModuleCest.php  |  2 +-
 .../Backend/Template/TemplateCest.php         |  6 +--
 6 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/typo3/sysext/backend/Resources/Public/JavaScript/PageTree/PageTree.js b/typo3/sysext/backend/Resources/Public/JavaScript/PageTree/PageTree.js
index bb8ad61cb085..de269ce253f8 100644
--- a/typo3/sysext/backend/Resources/Public/JavaScript/PageTree/PageTree.js
+++ b/typo3/sysext/backend/Resources/Public/JavaScript/PageTree/PageTree.js
@@ -109,7 +109,7 @@ define(['jquery',
       var title = TYPO3.lang.pagetree_networkErrorTitle;
       var desc = TYPO3.lang.pagetree_networkErrorDesc;
 
-      if (error && (error.target.status || error.target.statusText)) {
+      if (error && error.target && (error.target.status || error.target.statusText)) {
         title += ' - ' + (error.target.status || '')  + ' ' + (error.target.statusText || '');
       }
 
@@ -199,7 +199,7 @@ define(['jquery',
 
     PageTree.prototype.nodeRightClick = function (node) {
       d3.event.preventDefault();
-      var $node = $(node).closest('svg').find('.nodes .node[data-uid=' + this.identifier + ']');
+      var $node = $(node).closest('svg').find('.nodes .node[data-uid=' + this.stateIdentifier + ']');
 
       if ($node.length) {
         ContextMenu.show(
@@ -213,7 +213,7 @@ define(['jquery',
     };
 
     PageTree.prototype.contextmenu = function (node) {
-      var $node = $(node).closest('svg').find('.nodes .node[data-uid=' + this.identifier + ']');
+      var $node = $(node).closest('svg').find('.nodes .node[data-uid=' + this.stateIdentifier + ']');
 
       if ($node.length) {
         ContextMenu.show(
@@ -391,8 +391,8 @@ define(['jquery',
               _this.loadData();
             } else {
               node.name = node.newName;
-              _this.svg.select('.node-placeholder[data-uid="' + node.identifier + '"]').remove();
-              _this.update();
+              _this.svg.select('.node-placeholder[data-uid="' + node.stateIdentifier + '"]').remove();
+              _this.loadData();
               _this.nodesRemovePlaceholder();
             }
           } else {
diff --git a/typo3/sysext/backend/Resources/Public/JavaScript/PageTree/PageTreeDragDrop.js b/typo3/sysext/backend/Resources/Public/JavaScript/PageTree/PageTreeDragDrop.js
index 25c8c0bbc500..b171d0476c32 100644
--- a/typo3/sysext/backend/Resources/Public/JavaScript/PageTree/PageTreeDragDrop.js
+++ b/typo3/sysext/backend/Resources/Public/JavaScript/PageTree/PageTreeDragDrop.js
@@ -91,7 +91,7 @@ define([
           tree.settings.allowRecursiveDelete
         ) {
           _this.dropZoneDelete = tree.nodesContainer
-            .select('.node[data-uid="' + node.identifier + '"]')
+            .select('.node[data-uid="' + node.stateIdentifier + '"]')
             .append('g')
             .attr('class', 'nodes-drop-zone')
             .attr('height', tree.settings.nodeHeight);
@@ -139,7 +139,7 @@ define([
         var $svg = $(this).closest('svg');
         var $nodesBg = $svg.find('.nodes-bg');
         var $nodesWrap = $svg.find('.nodes-wrapper');
-        var $nodeBg = $nodesBg.find('.node-bg[data-uid=' + node.identifier + ']');
+        var $nodeBg = $nodesBg.find('.node-bg[data-uid=' + node.stateIdentifier + ']');
         var $nodeDd = $svg.siblings('.node-dd');
 
         if ($nodeBg.length && (!node.isDragged)) {
@@ -174,7 +174,7 @@ define([
         tree.settings.nodeDragPosition = false;
 
         if (node.isOver
-          || (tree.settings.nodeOver.node && tree.settings.nodeOver.node.parentsUid.indexOf(node.identifier) !== -1)
+          || (tree.settings.nodeOver.node && tree.settings.nodeOver.node.parentsUid.indexOf(node.stateIdentifier) !== -1)
           || !tree.isOverSvg) {
 
           _this.addNodeDdClass({ $nodeDd: $nodeDd, $nodesWrap: $nodesWrap, className: 'nodrop' });
@@ -250,7 +250,7 @@ define([
 
         if (
           !(node.isOver
-            || (tree.settings.nodeOver.node && tree.settings.nodeOver.node.parentsUid.indexOf(node.identifier) !== -1)
+            || (tree.settings.nodeOver.node && tree.settings.nodeOver.node.parentsUid.indexOf(node.stateIdentifier) !== -1)
             || !tree.settings.canNodeDrag
             || !tree.isOverSvg
           )
diff --git a/typo3/sysext/backend/Resources/Public/JavaScript/SvgTree.js b/typo3/sysext/backend/Resources/Public/JavaScript/SvgTree.js
index 6af870e72ffc..123ceb6c3fd1 100644
--- a/typo3/sysext/backend/Resources/Public/JavaScript/SvgTree.js
+++ b/typo3/sysext/backend/Resources/Public/JavaScript/SvgTree.js
@@ -196,6 +196,7 @@ define(
           .on('mouseout', function () {
             _this.isOverSvg = false;
           });
+
         this.container = this.svg
           .append('g')
           .attr('class', 'nodes-wrapper')
@@ -269,7 +270,7 @@ define(
             var title = TYPO3.lang.pagetree_networkErrorTitle;
             var desc = TYPO3.lang.pagetree_networkErrorDesc;
 
-            if (error.target.status || error.target.statusText) {
+            if (error && error.target && (error.target.status || error.target.statusText)) {
               title += ' - ' + (error.target.status || '')  + ' ' + (error.target.statusText || '');
             }
 
@@ -312,7 +313,7 @@ define(
               var currentNode = nodes[i];
               if (currentNode.depth < currentDepth) {
                 node.parents.push(i);
-                node.parentsUid.push(nodes[i].identifier);
+                node.parentsUid.push(nodes[i].stateIdentifier);
                 currentDepth = currentNode.depth;
               }
             }
@@ -320,12 +321,18 @@ define(
             node.open = true;
           }
 
+          // create stateIdentifier if doesn't exist (for category tree)
+          if (!node.stateIdentifier) {
+            var parentId = (node.parents.length) ? node.parents[node.parents.length - 1] : node.identifier;
+            node.stateIdentifier = parentId + '_' + node.identifier;
+          }
+
           if (typeof node.checked === 'undefined') {
             node.checked = false;
             _this.settings.unselectableElements.push(node.identifier);
           }
 
-          //dispatch event
+          // dispatch event
           _this.dispatch.call('prepareLoadedNode', _this, node);
           return node;
         });
@@ -371,7 +378,7 @@ define(
         var pathAboveMounts = 0;
 
         this.data.nodes.forEach(function (n, i) {
-          //delete n.children;
+          // delete n.children;
           n.x = n.depth * _this.settings.indentWidth;
 
           if (n.readableRootline) {
@@ -436,11 +443,11 @@ define(
 
         var visibleNodes = this.data.nodes.slice(position, position + visibleRows);
         var nodes = this.nodesContainer.selectAll('.node').data(visibleNodes, function (d) {
-          return d.identifier;
+          return d.stateIdentifier;
         });
 
         var nodesBg = this.nodesBgContainer.selectAll('.node-bg').data(visibleNodes, function (d) {
-          return d.identifier;
+          return d.stateIdentifier;
         });
 
         // delete nodes without corresponding data
@@ -488,7 +495,7 @@ define(
             .attr('xlink:href', this.getIconOverlayId);
         }
 
-        //dispatch event
+        // dispatch event
         this.dispatch.call('updateNodes', this, nodes);
       },
 
@@ -504,7 +511,7 @@ define(
           .merge(nodesBg)
           .attr('width', '100%')
           .attr('height', this.settings.nodeHeight)
-          .attr('data-uid', this.getNodeIdentifier)
+          .attr('data-uid', this.getNodeStateIdentifier)
           .attr('transform', this.getNodeBgTransform)
           .on('mouseover', function (node) {
             _this.nodeBgEvents().mouseOver(node, this);
@@ -530,7 +537,7 @@ define(
         var self = {};
 
         self.mouseOver = function (node, element) {
-          var elementNodeBg = _this.svg.select('.nodes-bg .node-bg[data-uid="' + node.identifier + '"]');
+          var elementNodeBg = _this.svg.select('.nodes-bg .node-bg[data-uid="' + node.stateIdentifier + '"]');
 
           node.isOver = true;
           _this.settings.nodeOver.node = node;
@@ -544,7 +551,7 @@ define(
         };
 
         self.mouseOut = function (node, element) {
-          var elementNodeBg = _this.svg.select('.nodes-bg .node-bg[data-uid="' + node.identifier + '"]');
+          var elementNodeBg = _this.svg.select('.nodes-bg .node-bg[data-uid="' + node.stateIdentifier + '"]');
 
           node.isOver = false;
           _this.settings.nodeOver.node = false;
@@ -558,7 +565,7 @@ define(
         };
 
         self.click = function (node, element) {
-          var $nodeBg = $(element).closest('svg').find('.nodes-bg .node-bg[data-uid=' + node.identifier + ']');
+          var $nodeBg = $(element).closest('svg').find('.nodes-bg .node-bg[data-uid=' + node.stateIdentifier + ']');
 
           _this.nodes.forEach(function (node) {
             if (node.selected === true) {
@@ -597,12 +604,12 @@ define(
           .exit()
           .remove();
 
-        //create
+        // create
         links.enter()
           .append('path')
           .attr('class', 'link')
 
-          //create + update
+          // create + update
           .merge(links)
           .attr('d', this.getLinkPath.bind(_this));
       },
@@ -625,7 +632,7 @@ define(
           var icons = this.iconsContainer
             .selectAll('.icon-def')
             .data(iconsArray, function (i) {
-              return i.identifier;
+              return i.stateIdentifier;
             });
 
           icons
@@ -636,7 +643,7 @@ define(
               return 'icon-' + i.identifier;
             })
             .append(function (i) {
-              //workaround for IE11 where you can't simply call .html(content) on svg
+              // workaround for IE11 where you can't simply call .html(content) on svg
               var parser = new DOMParser();
               var markupText = i.icon.replace('<svg', '<g').replace('/svg>', '/g>');
               markupText = "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'>" + markupText + '</svg>';
@@ -734,7 +741,7 @@ define(
           .attr('class', this.getNodeClass)
           .attr('transform', this.getNodeTransform)
           .attr('data-table', 'pages')
-          .attr('data-uid', this.getNodeIdentifier)
+          .attr('data-uid', this.getNodeStateIdentifier)
           .attr('title', this.getNodeTitle)
           .on('mouseover', function (node) {
             _this.nodeBgEvents().mouseOver(node, this);
@@ -771,6 +778,16 @@ define(
         return node.identifier;
       },
 
+      /**
+       * Computes the tree item state identifier based on the data
+       *
+       * @param {Node} node
+       * @returns {String}
+       */
+      getNodeStateIdentifier: function (node) {
+        return node.stateIdentifier;
+      },
+
       /**
        * Computes the tree item label based on the data
        *
@@ -788,7 +805,7 @@ define(
        * @returns {String}
        */
       getNodeClass: function (node) {
-        return 'node identifier-' + node.identifier;
+        return 'node identifier-' + node.stateIdentifier;
       },
 
       /**
@@ -991,7 +1008,7 @@ define(
             this.exclusiveSelectedNode = node;
           } else if (exclusiveKeys.indexOf('' + node.identifier) === -1 && this.exclusiveSelectedNode) {
 
-            //current node is not exclusive, but other exclusive node is already selected
+            // current node is not exclusive, but other exclusive node is already selected
             this.exclusiveSelectedNode.checked = false;
             this.dispatch.call('nodeSelectedAfter', this, this.exclusiveSelectedNode);
             this.exclusiveSelectedNode = null;
diff --git a/typo3/sysext/core/Tests/Acceptance/Backend/Formhandler/CategoryTreeCest.php b/typo3/sysext/core/Tests/Acceptance/Backend/Formhandler/CategoryTreeCest.php
index 1070795fbac2..797dd5e8bc3e 100644
--- a/typo3/sysext/core/Tests/Acceptance/Backend/Formhandler/CategoryTreeCest.php
+++ b/typo3/sysext/core/Tests/Acceptance/Backend/Formhandler/CategoryTreeCest.php
@@ -59,13 +59,13 @@ class CategoryTreeCest
         $I->executeJS('$(\'.icon-actions-view-list-collapse\').click();');
         $I->wait(1);
         $I->executeJS('$(\'a[data-table="sys_category"] .icon-actions-view-list-expand\').click();');
-        $I->wait(1);
+        $I->waitForElementVisible('#recordlist-sys_category tr[data-uid="7"] a[data-original-title="Edit record"]');
         // Select category with id 7
         $I->click('#recordlist-sys_category tr[data-uid="7"] a[data-original-title="Edit record"]');
         // Change title and level to root
         $I->fillField('input[data-formengine-input-name="data[sys_category][7][title]"]', 'level-1-4');
-        $I->click('.identifier-7 text.node-name');
-        $I->click('.identifier-3 text.node-name');
+        $I->click('.identifier-0_7 text.node-name');
+        $I->click('.identifier-0_3 text.node-name');
         $I->click('button[name="_savedok"]');
         // Wait for tree and check if isset level-1-4
         $I->waitForElement('.svg-tree-wrapper svg');
diff --git a/typo3/sysext/core/Tests/Acceptance/Backend/Page/AddPageInPageModuleCest.php b/typo3/sysext/core/Tests/Acceptance/Backend/Page/AddPageInPageModuleCest.php
index 5df8c2be1a9f..29e0779fd8a1 100644
--- a/typo3/sysext/core/Tests/Acceptance/Backend/Page/AddPageInPageModuleCest.php
+++ b/typo3/sysext/core/Tests/Acceptance/Backend/Page/AddPageInPageModuleCest.php
@@ -49,7 +49,7 @@ class AddPageInPageModuleCest
         $typo3NavigationContainer = '.scaffold-content-navigation-component';
         $I->waitForElement($typo3NavigationContainer);
         $rootNode = 'a.x-tree-node-anchor > span';
-        $rootNodeIcon = '.node.identifier-0 .node-icon';
+        $rootNodeIcon = '.node.identifier-0_0 .node-icon';
         $rootNodeContextMenuMore = '#contentMenu0 a.list-group-item-submenu';
         //create new wizard
         $contextMenuNew = '#contentMenu1 .list-group-item[data-callback-action=newPageWizard]';
diff --git a/typo3/sysext/core/Tests/Acceptance/Backend/Template/TemplateCest.php b/typo3/sysext/core/Tests/Acceptance/Backend/Template/TemplateCest.php
index 018e0a87b142..54e2fe2bba0e 100644
--- a/typo3/sysext/core/Tests/Acceptance/Backend/Template/TemplateCest.php
+++ b/typo3/sysext/core/Tests/Acceptance/Backend/Template/TemplateCest.php
@@ -49,7 +49,7 @@ class TemplateCest
         $I->wantTo('show templates overview on root page (uid = 0)');
         $I->switchToIFrame();
         // click on root page
-        $I->click('.node.identifier-0');
+        $I->click('.node.identifier-0_0');
         $I->switchToIFrame('list_frame');
         $I->waitForElementVisible('#ts-overview');
         $I->see('This is an overview of the pages in the database containing one or more template records. Click a page title to go to the page.');
@@ -57,7 +57,7 @@ class TemplateCest
         $I->wantTo('show templates overview on website root page (uid = 1 and pid = 0)');
         $I->switchToIFrame();
         // click on website root page
-        $I->click('.node.identifier-1');
+        $I->click('.node.identifier-0_1');
         $I->switchToIFrame('list_frame');
         $I->waitForText('No template');
         $I->see('There was no template on this page!');
@@ -80,7 +80,7 @@ class TemplateCest
     {
         $I->wantTo('create a new site template');
         $I->switchToIFrame();
-        $I->click('.node.identifier-1');
+        $I->click('.node.identifier-0_1');
         $I->switchToIFrame('list_frame');
         $I->waitForText('Create new website');
         $I->click("//input[@name='newWebsite']");
-- 
GitLab