* bug #14296 fixed - Xcos labels crashed Scilab. 49/20249/3
Clément DAVID [Thu, 29 Mar 2018 13:35:32 +0000 (15:35 +0200)]
f3ceb6ca introduced the DESCRIPTION fields for blocks but did not ensure
that this field contains only C / Scilab compatible names.

Change-Id: I9369a2ef73aaba09c78346c56878289911b73b22

scilab/CHANGES.md
scilab/modules/graph/src/java/org/scilab/modules/graph/ScilabCanvas.java
scilab/modules/scicos/src/cpp/view_scilab/GraphicsAdapter.cpp
scilab/modules/scicos/src/cpp/view_scilab/ModelAdapter.cpp
scilab/modules/xcos/src/java/org/scilab/modules/xcos/Xcos.java
scilab/modules/xcos/src/java/org/scilab/modules/xcos/actions/EditFormatAction.java
scilab/modules/xcos/src/java/org/scilab/modules/xcos/block/AfficheBlock.java
scilab/modules/xcos/src/java/org/scilab/modules/xcos/graph/model/XcosCell.java
scilab/modules/xcos/src/java/org/scilab/modules/xcos/io/sax/BlockHandler.java

index 15b28de..ee4cf8d 100644 (file)
@@ -417,6 +417,7 @@ Known issues
 * [#13915](http://bugzilla.scilab.org/show_bug.cgi?id=13915): On Windows, reinstalling an ATOMS toolbox with an archive already present failed.
 * [#14010](http://bugzilla.scilab.org/show_bug.cgi?id=14010): Browsevar was not displaying dimensions > 2 of hypermatrix
 * [#14218](http://bugzilla.scilab.org/show_bug.cgi?id=14218): Karmarkar post-process fails on some problems.
+* [#14296](http://bugzilla.scilab.org/show_bug.cgi?id=14296): Xcos labels moving after creating blocks using scripts crashed Scilab.
 * [#14460](http://bugzilla.scilab.org/show_bug.cgi?id=14460): sparse boolean indices were not supported.
 * [#14489](http://bugzilla.scilab.org/show_bug.cgi?id=14489): clicking the scinotes icon did not bring its window in the foreground.
 * [#14521](http://bugzilla.scilab.org/show_bug.cgi?id=14521): For the CONVERT Xcos block, types codes 2, 6, 7, and 8 were not documented.
index f3acdf9..adaa573 100644 (file)
@@ -1,6 +1,7 @@
 /*
  * Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
  * Copyright (C) 2009 - DIGITEO - Clement DAVID
+ * Copyright (C) 2018 - ESI Group - Clement DAVID
  *
  * Copyright (C) 2012 - 2016 - Scilab Enterprises
  *
@@ -41,6 +42,7 @@ import com.mxgraph.util.mxConstants;
 import com.mxgraph.util.mxRectangle;
 import com.mxgraph.util.mxUtils;
 import com.mxgraph.view.mxCellState;
+import java.awt.geom.NoninvertibleTransformException;
 
 /**
  * Painter for each vertex and edge
@@ -299,7 +301,11 @@ public class ScilabCanvas extends mxInteractiveCanvas {
         g.scale(ratio, ratio);
 
         // Paint
-        icon.paint(g);
+        try {
+            icon.paint(g);
+        } catch (IllegalArgumentException e) { // or NoninvertibleTransformException
+            // sometimes rethrow after a NoninvertibleTransformException.
+        }
     }
 
     /**
index b9b798c..d4c1a34 100644 (file)
@@ -442,14 +442,23 @@ struct id
 
     static types::InternalType* get(const GraphicsAdapter& adaptor, const Controller& controller)
     {
-        ScicosID adaptee = adaptor.getAdaptee()->id();
+        model::Block* adaptee = adaptor.getAdaptee();
 
-        std::string id;
-        controller.getObjectProperty(adaptee, BLOCK, DESCRIPTION, id);
+        ScicosID label;
+        std::string description;
 
-        types::String* o = new types::String(1, 1);
-        o->set(0, id.data());
+        controller.getObjectProperty(adaptee, LABEL, label);
+        if (label != ScicosID())
+        {
+            controller.getObjectProperty(label, ANNOTATION, DESCRIPTION, description);
+        }
+        else
+        {
+            controller.getObjectProperty(adaptee, DESCRIPTION, description);
+        }
 
+        types::String* o = new types::String(1, 1);
+        o->set(0, description.data());
         return o;
     }
 
@@ -468,13 +477,22 @@ struct id
             return false;
         }
 
-        ScicosID adaptee = adaptor.getAdaptee()->id();
-
         char* c_str = wide_string_to_UTF8(current->get(0));
-        std::string id(c_str);
+        std::string description(c_str);
         FREE(c_str);
 
-        controller.setObjectProperty(adaptee, BLOCK, DESCRIPTION, id);
+        model::Block* adaptee = adaptor.getAdaptee();
+
+        ScicosID label;
+        controller.getObjectProperty(adaptee, LABEL, label);
+        if (label != ScicosID())
+        {
+            controller.setObjectProperty(label, ANNOTATION, DESCRIPTION, description);
+        }
+        else
+        {
+            controller.setObjectProperty(adaptee, DESCRIPTION, description);
+        }
         return true;
     }
 };
index d9ea9b0..5fbd1b3 100644 (file)
@@ -1179,30 +1179,12 @@ struct label
     {
         model::Block* adaptee = adaptor.getAdaptee();
 
-        ScicosID label;
         std::string description;
-
-        controller.getObjectProperty(adaptee, LABEL, label);
-        if (label != ScicosID())
-        {
-            controller.getObjectProperty(label, ANNOTATION, DESCRIPTION, description);
-        }
-        else
-        {
-            controller.getObjectProperty(adaptee, DESCRIPTION, description);
-        }
+        controller.getObjectProperty(adaptee, DESCRIPTION, description);
 
         types::String* o = new types::String(1, 1);
+        o->set(0, description.data());
 
-        // safety check ; the returned value should always be a valid C / modelica identifier
-        if (isValidCIdentifier(description))
-        {
-            o->set(0, description.data());
-        }
-        else
-        {
-            o->set(0, "");
-        }
         return o;
     }
 
@@ -1227,13 +1209,6 @@ struct label
         std::string description(c_str);
         FREE(c_str);
 
-        // TODO: validate a C/Scilab identifier only
-        //        if (!isValidCIdentifier(label))
-        //        {
-        //            get_or_allocate_logger()->log(LOG_ERROR, _("Wrong value for field %s.%s : Valid C identifier expected.\n"), "model", "label");
-        //            return false;
-        //        }
-
         controller.setObjectProperty(adaptee, DESCRIPTION, description);
         return true;
     }
index 61b606d..a4514c8 100644 (file)
@@ -423,7 +423,7 @@ public final class Xcos {
             }
         }
 
-        final long currentId;
+        long currentId;
         if (diagramId != 0) {
             currentId = diagramId;
         } else {
@@ -442,7 +442,27 @@ public final class Xcos {
             /*
              * Allocate and setup a new diagram
              */
-            diag = new XcosDiagram(controller, currentId, controller.getKind(currentId), "");
+            Kind currentKind = controller.getKind(currentId);
+            if (Kind.BLOCK.equals(currentKind)) {
+                long[] rootId = new long[1];
+                controller.getObjectProperty(currentId, currentKind, ObjectProperties.PARENT_DIAGRAM, rootId);
+
+                if (rootId[0] == 0) {
+                    // a SuperBlock without a parent diagram, duplicate it as a root diagram
+                    VectorOfScicosID children = new VectorOfScicosID();
+                    controller.getObjectProperty(currentId, currentKind, ObjectProperties.CHILDREN, children);
+
+                    currentKind = Kind.DIAGRAM;
+                    currentId = controller.createObject(Kind.DIAGRAM);
+                    controller.setObjectProperty(currentId, currentKind, ObjectProperties.CHILDREN, children);
+
+                    for (int i = 0; i < children.size(); i++) {
+                        controller.referenceObject(children.get(i));
+                    }
+                }
+            }
+
+            diag = new XcosDiagram(controller, currentId, currentKind, "");
             diag.installListeners();
 
             root = findRoot(controller, diag);
index 93a4e69..3275c68 100644 (file)
@@ -2,7 +2,7 @@
  * Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
  * Copyright (C) 2010-2011 - DIGITEO - Clement DAVID
  * Copyright (C) 2011-2015 - Scilab Enterprises - Clement DAVID
- * Copyright (C) 2017 - ESI Group - Clement DAVID
+ * Copyright (C) 2017-2018 - ESI Group - Clement DAVID
  *
  * Copyright (C) 2012 - 2016 - Scilab Enterprises
  *
@@ -43,7 +43,6 @@ import org.scilab.modules.graph.actions.base.DefaultAction;
 import org.scilab.modules.graph.utils.StyleMap;
 import org.scilab.modules.gui.menuitem.MenuItem;
 import org.scilab.modules.gui.utils.ScilabSwingUtilities;
-import org.scilab.modules.xcos.block.SuperBlock;
 import org.scilab.modules.xcos.block.TextBlock;
 import org.scilab.modules.xcos.graph.XcosDiagram;
 import org.scilab.modules.xcos.utils.XcosMessages;
@@ -356,6 +355,21 @@ public final class EditFormatAction extends DefaultAction {
             model.setStyle(identifier, identifierStyle.toString());
         }
 
+        // convert to a C / Scilab compatible variable name
+        StringBuilder str = new StringBuilder(oneliner.length());
+        for (char c : oneliner.toCharArray()) {
+            if (('0' <= c && c <= '9') || ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') || ( c == '_')) {
+                str.append(c);
+            }
+            if (c == ' ') {
+                str.append('_');
+            }
+        }
+        if (str.length() > 0 && '0' <= str.charAt(0) && str.charAt(0) <= '9') {
+            str.insert(0, '_');
+        }
+        oneliner = str.toString();
+
         graph.cellLabelChanged(cell, oneliner, false);
         graph.fireEvent(new mxEventObject(mxEvent.LABEL_CHANGED, "cell", cell, "value", text, "parent", cell.getParent()));
 
index ffa7f96..6908555 100644 (file)
@@ -130,6 +130,24 @@ public final class AfficheBlock extends BasicBlock {
         super(controller, uid, kind, value, geometry, style, id);
     }
 
+    /**
+     * As the displayed text is stored as block's value, this implementation
+     * must ensure that this text is stored properly even if it is not a valid
+     * Scilab / C variable name.
+     *
+     * @param controller the controller
+     * @param value the value
+     */
+    @Override
+    protected void setMVCValue(JavaController controller, Object value) {
+        if (value == null) {
+            return;
+        }
+
+        // don't check anything, let the user handle that
+        controller.setObjectProperty(getUID(), getKind(), ObjectProperties.DESCRIPTION, String.valueOf(value));
+    }
+
     @Override
     protected void updateStyle(JavaController controller, XcosDiagram parent, BasicBlock modifiedBlock) {
         final StyleMap styleMap = new StyleMap(modifiedBlock.getStyle());
index 385a682..982483d 100644 (file)
@@ -46,7 +46,7 @@ import org.scilab.modules.xcos.block.TextBlock;
  */
 public class XcosCell extends mxCell {
     private static final long serialVersionUID = 1L;
-    private static final Pattern validCIdentifier = Pattern.compile("[a-zA-Z][a-zA-Z0-9_]+");
+    private static Pattern validCIdentifier = null;
 
     private transient ScicosObjectOwner owner;
 
@@ -108,17 +108,26 @@ public class XcosCell extends mxCell {
         setMVCValue(controller, value);
     }
 
+    /**
+     * Store the value on the C++ model.
+     * @param controller the controller
+     * @param value the value to store
+     */
     @SuppressWarnings("fallthrough")
-    private void setMVCValue(JavaController controller, Object value) {
+    protected void setMVCValue(JavaController controller, Object value) {
         if (value == null) {
             return;
         }
 
         switch (getKind()) {
             case BLOCK:
-                if (validCIdentifier.matcher(String.valueOf(value)).matches()) {
+                if (validCIdentifier == null) {
+                    validCIdentifier = Pattern.compile("[a-zA-Z_][a-zA-Z0-9_]*");
+                }
+                String description = String.valueOf(value);
+                if (validCIdentifier.matcher(description).matches()) {
                     // a block description should be a valid C / Scilab identifier to ease codegeneration
-                    controller.setObjectProperty(getUID(), getKind(), ObjectProperties.DESCRIPTION, String.valueOf(value));
+                    controller.setObjectProperty(getUID(), getKind(), ObjectProperties.DESCRIPTION, description);
                 }
                 break;
             case ANNOTATION:
index aa79150..7acee5e 100644 (file)
@@ -17,6 +17,7 @@
 package org.scilab.modules.xcos.io.sax;
 
 import java.util.Arrays;
+import java.util.regex.Pattern;
 
 import org.scilab.modules.xcos.Kind;
 import org.scilab.modules.xcos.ObjectProperties;
@@ -44,6 +45,7 @@ import org.scilab.modules.xcos.io.HandledElement;
 import org.xml.sax.Attributes;
 
 class BlockHandler implements ScilabHandler {
+    private static Pattern validCIdentifier = null;
 
     private final XcosSAXHandler saxHandler;
 
@@ -68,7 +70,10 @@ class BlockHandler implements ScilabHandler {
         final long uid = saxHandler.controller.createObject(kind);
 
         String value = atts.getValue("value");
-        if (value != null) {
+        if (validCIdentifier == null) {
+            validCIdentifier = Pattern.compile("[a-zA-Z_][a-zA-Z0-9_]*");
+        }
+        if (value != null && validCIdentifier.matcher(value).matches()) {
             saxHandler.controller.setObjectProperty(uid, kind, ObjectProperties.DESCRIPTION, value);
         }