Network: Add extra check on null value during label handling (#3511)

* Network: Add extra check on null value during label handling

This fixes an oversight in #3486. Unit tests added, not only for null labels, but for all weird label values I could thing of.

* Enhanced unit tests, adjusted label check

lib/network/modules/components/Edge.js

@@ -1,5 +1,4 @@
 var util = require('../../../util');
-
 var Label = require('./shared/Label').default;
 var ComponentUtil = require('./shared/ComponentUtil').default;
 var CubicBezierEdge = require('./edges/CubicBezierEdge').default;
@@ -7,6 +6,7 @@ var BezierEdgeDynamic = require('./edges/BezierEdgeDynamic').default;
 var BezierEdgeStatic = require('./edges/BezierEdgeStatic').default;
 var StraightEdge = require('./edges/StraightEdge').default;
 
+
 /**
  * An edge connects two nodes and has a specific direction.
  */
@@ -118,7 +118,6 @@ class Edge {
       'from',
       'hidden',
       'hoverWidth',
-      'label',
       'labelHighlightBold',
       'length',
       'line',
@@ -138,6 +137,13 @@ class Edge {
     // only deep extend the items in the field array. These do not have shorthand.
     util.selectiveDeepExtend(fields, parentOptions, newOptions, allowDeletion);
 
+    // Only copy label if it's a legal value.
+    if (ComponentUtil.isValidLabel(newOptions.label)) {
+      parentOptions.label = newOptions.label;
+    } else {
+      parentOptions.label = undefined;
+    }
+
     util.mergeOptions(parentOptions, newOptions, 'smooth', globalOptions);
     util.mergeOptions(parentOptions, newOptions, 'shadow', globalOptions);
 

lib/network/modules/components/shared/ComponentUtil.js

@@ -122,6 +122,18 @@ class ComponentUtil {
       bottom    > point.y
     );
   }
+
+
+  /**
+   * Check if given value is acceptable as a label text.
+   *
+   * @param {*} text value to check; can be anything at this point
+   * @returns {boolean} true if valid label value, false otherwise
+   */
+  static isValidLabel(text) {
+    // Note that this is quite strict: types that *might* be converted to string are disallowed
+    return  (typeof text === 'string' && text !== '');
+  }
 }
 
 export default ComponentUtil;

lib/network/modules/components/shared/Label.js

@@ -64,8 +64,11 @@ class Label {
 
     this.initFontOptions(options.font);
 
-    if (options.label !== undefined && options.label !== null && options.label !== '') {
+    if (ComponentUtil.isValidLabel(options.label)) {
       this.labelDirty = true;
+    } else {
+      // Bad label! Change the option value to prevent bad stuff happening
+      options.label = '';
     }
 
     if (options.font !== undefined && options.font !== null) { // font options can be deleted at various levels

lib/network/modules/components/shared/LabelSplitter.js

@@ -1,4 +1,5 @@
 let LabelAccumulator = require('./LabelAccumulator').default;
+let ComponentUtil = require('./ComponentUtil').default;
 
 
 /**
@@ -67,7 +68,7 @@ class LabelSplitter {
    * @returns {Array<line>}
    */
   process(text) {
-    if (text === undefined || text === "") {
+    if (!ComponentUtil.isValidLabel(text)) {
       return this.lines.finalize();
     }
 

test/Label.test.js

@@ -1326,7 +1326,6 @@ describe('Shorthand Font Options', function() {
 
     done();
   });
-
 });  // Multi-Fonts
 
 
@@ -1407,4 +1406,75 @@ describe('Shorthand Font Options', function() {
 
     done();
   });
+
+
+  it('deals with null labels and other awkward values', function (done) {
+    var ctx = new DummyContext();
+    var options = getOptions({});
+
+    var checkHandling = (label, index, text) => {
+      assert.doesNotThrow(() => {label.getTextSize(ctx, false, false)}, "Unexpected throw for " + text + " " + index);
+      //label.getTextSize(ctx, false, false);  // Use this to determine the error thrown
+
+      // There should not be a label for any of the cases
+      // 
+      let labelVal = label.elementOptions.label;
+      let validLabel = (typeof labelVal === 'string' && labelVal !== '');
+      assert(!validLabel, "Unexpected label value '" + labelVal+ "' for " + text +" " + index);
+    };
+
+    var nodes = [
+      {id: 1},
+      {id: 2, label: null},
+      {id: 3, label: undefined},
+      {id: 4, label: {a: 42}},
+      {id: 5, label: [ 'an', 'array']},
+      {id: 6, label: true},
+      {id: 7, label: 3.419},
+    ];
+
+    var edges = [
+      {from: 1, to: 2, label: null},
+      {from: 1, to: 3, label: undefined},
+      {from: 1, to: 4, label: {a: 42}},
+      {from: 1, to: 5, label: ['an', 'array']},
+      {from: 1, to: 6, label: false},
+      {from: 1, to: 7, label: 2.71828},
+    ];
+
+    // Isolate the specific call where a problem with null-label was detected
+    // Following loops should plain not throw
+
+
+    // Node labels
+    for (let i = 0; i < nodes.length; ++i) {
+      let label = new Label(null, nodes[i], false);
+      checkHandling(label, i, 'node');
+    }
+
+
+    // Edge labels
+    for (let i = 0; i < edges.length; ++i) {
+      let label = new Label(null, edges[i], true);
+      checkHandling(label, i, 'edge');
+    }
+
+
+    //
+    // Following extracted from example 'nodeLegend', where the problem was detected.
+    // 
+    // In the example, only `label:null` was present. The weird thing is that it fails
+    // in the example, but succeeds in the unit tests.
+    // Kept in for regression testing.
+    var container = document.getElementById('mynetwork');
+    var data = {
+      nodes: new vis.DataSet(nodes),
+      edges: new vis.DataSet(edges)
+    };
+
+    var options = {};
+    var network = new vis.Network(container, data, options);
+
+    done();
+  });
 });