math.makeBoundingBox() can return null

[trombonehero]

Bug report

Environment info

• Cytoscape.js version : 3.2.7
• Browser/Node.js & version : Chrome 66.0.3359.181

Current (buggy) behaviour

The function math.makeBoundingBox can return null when the input bb has w: NaN and no x2 or y2. I see this happening when creating a new cytoscape instance in Chrome in an as-yet-unsized DOM node (i.e., width = height = 0), where cy.width() and cy.height() return NaN:

  | math.makeBoundingBox | @ | cytoscape.cjs.js:598
  | GridLayout.run | @ | cytoscape.cjs.js:15689
  | setElesAndLayout | @ | cytoscape.cjs.js:3280
  | (anonymous) | @ | cytoscape.cjs.js:3293
  | loadExtData | @ | cytoscape.cjs.js:3225
  | Core | @ | cytoscape.cjs.js:3283
  | cytoscape | @ | cytoscape.cjs.js:25436

This leads to an uncaught TypeError in Chrome. It doesn't seem to cause the same problem in Firefox...

Desired behaviour

I expected not to see a TypeError and subsequent stoppage of the webpage.

Minimum steps to reproduce

This is a bit tricky to reproduce without bringing in lots of context (e.g., Marko components). I think that the fix is pretty simple though:

--- src/core/viewport.old.js    2018-05-25 14:59:22.000000000 -0230
+++ src/core/viewport.js        2018-05-25 14:56:35.000000000 -0230
@@ -523,11 +523,13 @@
   },

   width: function(){
-    return this.size().width;
+    var w = this.size().width;
+    return w ? w : 0;
   },

   height: function(){
-    return this.size().height;
+    var h = this.size().height;
+    return h ? h : 0;
   },

   extent: function(){

Or, alternatively, add another return { x1: Infinity /* ... */ } to the end of math.makeBoundingBox().

[maxkfranz]

Are you initialising after DOMContentLoaded on a div in the document tree?

[trombonehero]

That's a good question... I'm initialising when my Marko component "mounts" into the DOM: https://markojs.com/docs/components/#codeonmountcode

The div element does exist at this point, but I'm not sure if DOMContentLoaded has fired yet. Is there a simple way to check?

Just for reference, this issue occurs on Chrome but not Firefox.

[maxkfranz]

Is there a simple way to check?

Log when loaded happens versus your mount

[trombonehero]

Ok, yes: DOMContentLoaded fires before this error occurs. I don't see DOMContentReady, but I do see DOMContentLoaded before the "mount" event.

[trombonehero]

Going slightly further upstream, there is another potential fix for my immediate issue:

--- src/core/viewport.orig.js   2018-05-30 14:36:50.000000000 -0230
+++ src/core/viewport.js        2018-05-30 14:36:34.000000000 -0230
@@ -510,7 +510,10 @@

     return ( _p.sizeCache = _p.sizeCache || ( container ? (function(){
       let style = window.getComputedStyle( container );
-      let val = function( name ){ return parseFloat( style.getPropertyValue( name ) ); };
+      let val = function( name ){
+        var str = style.getPropertyValue( name );
+        return (str == "") ? 0 : parseFloat(str);
+      };

       return {
         width: container.clientWidth - val('padding-left') - val('padding-right'),

However, I still think it'd be a good idea to ensure that all code paths through https://github.com/cytoscape/cytoscape.js/blob/master/src/math.js#L118 return something...

[maxkfranz]

A zero area container does not make sense for a rendered instance. Adding workarounds to bounding box code doesn't sit right with me. It's just kicking the can farther down the road: Zero or infinite values aren't going to make the graph render properly or layout properly. It's a case of garbage in, garbage out.

Are you initialising after DOMContentLoaded on a div in the document tree?

If the first condition is true, the second is probably not. This seems like it may be a problem with the framework you're using. Perhaps it's giving you a pre-mount hook rather than a post-mount hook.

I don't think there's anything on this side that can properly resolve your issue. If you can build a minimal demo (no framework) that shows Cytoscape failing, please re-open this issue with the link.