fix: bug when Text and shape using same clipPath

fix and close apache/echarts#6989 Mostly because `appendChild` the same DOM element to different parents, making the previous appending invalid
ecomfe · Nov 13, 2017 · c05ac82 · c05ac82
1 parent 69e16cf
commit c05ac82
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 9 deletions.
diff --git a/src/svg/Painter.js b/src/svg/Painter.js
@@ -154,11 +154,9 @@ SVGPainter.prototype = {
             if (!displayable.invisible) {
                 if (displayable.__dirty) {
                     svgProxy && svgProxy.brush(displayable);
-                    var el = getSvgElement(displayable)
-                        || getTextSvgElement(displayable);
 
                     // Update clipPath
-                    this.clipPathManager.update(displayable, el);
+                    this.clipPathManager.update(displayable);
 
                     // Update gradient
                     if (displayable.style) {

diff --git a/src/svg/graphic.js b/src/svg/graphic.js
@@ -334,10 +334,13 @@ var svgTextDrawRectText = function (el, rect, textRect) {
 
     var text = style.text;
     // Convert to string
-    text != null && (text += '');
-    if (!text) {
+    if (text == null) {
+        // Draw no text only when text is set to null, but not ''
         return;
     }
+    else {
+        text += '';
+    }
 
     var textSvgEl = el.__textSvgEl;
     if (! textSvgEl) {

diff --git a/src/svg/helper/ClippathManager.js b/src/svg/helper/ClippathManager.js
@@ -26,10 +26,12 @@ zrUtil.inherits(ClippathManager, Definable);
  * Update clipPath.
  *
  * @param {Displayable} displayable displayable element
- * @param {SVGElement}  svgElement  SVG element of displayable
  */
-ClippathManager.prototype.update = function (displayable, svgElement) {
-    this.updateDom(svgElement, displayable.__clipPaths, false);
+ClippathManager.prototype.update = function (displayable) {
+    var svgEl = this.getSvgElement(displayable);
+    if (svgEl) {
+        this.updateDom(svgEl, displayable.__clipPaths, false);
+    }
 
     var textEl = this.getTextSvgElement(displayable);
     if (textEl) {
@@ -121,7 +123,13 @@ ClippathManager.prototype.updateDom = function (
         }
 
         var pathEl = this.getSvgElement(clipPath);
-        clipPathEl.appendChild(pathEl);
+        /**
+         * Use `cloneNode()` here to appendChild to multiple parents,
+         * which may happend when Text and other shapes are using the same
+         * clipPath. Since Text will create an extra clipPath DOM due to
+         * different transform rules.
+         */
+        clipPathEl.appendChild(pathEl.cloneNode());
 
         parentEl.setAttribute('clip-path', 'url(#' + id + ')');