Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

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

Merged
merged 2 commits into from
Oct 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions lib/network/modules/components/Edge.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
var util = require('../../../util');

var Label = require('./shared/Label').default;
var ComponentUtil = require('./shared/ComponentUtil').default;
var CubicBezierEdge = require('./edges/CubicBezierEdge').default;
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.
*/
Expand Down Expand Up @@ -118,7 +118,6 @@ class Edge {
'from',
'hidden',
'hoverWidth',
'label',
'labelHighlightBold',
'length',
'line',
Expand All @@ -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);

Expand Down
12 changes: 12 additions & 0 deletions lib/network/modules/components/shared/ComponentUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
5 changes: 4 additions & 1 deletion lib/network/modules/components/shared/Label.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/network/modules/components/shared/LabelSplitter.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
let LabelAccumulator = require('./LabelAccumulator').default;
let ComponentUtil = require('./ComponentUtil').default;


/**
Expand Down Expand Up @@ -67,7 +68,7 @@ class LabelSplitter {
* @returns {Array<line>}
*/
process(text) {
if (text === undefined || text === "") {
if (!ComponentUtil.isValidLabel(text)) {
return this.lines.finalize();
}

Expand Down
72 changes: 71 additions & 1 deletion test/Label.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1326,7 +1326,6 @@ describe('Shorthand Font Options', function() {

done();
});

}); // Multi-Fonts


Expand Down Expand Up @@ -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();
});
});