Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loading a large SVG graph reports an error Cannot read property 'naturalWidth' of null #6070

Closed
MrChuJiu opened this issue Jan 5, 2020 · 4 comments · Fixed by #6285
Closed

Comments

@MrChuJiu
Copy link

MrChuJiu commented Jan 5, 2020

Version

3.6.0

Test Case

http://jsfiddle.net/fabricjs/Da7SP/

Information about environment

Nodejs or browser?
Which browsers?

Steps to reproduce

Expected Behavior

Actual Behavior

image

image

image.zip

@asturur
Copy link
Member

asturur commented Jan 5, 2020

The problem is that images do not have a a link to a real image.
And fabricJS crashes. We could avoid crashing.

@MrChuJiu
Copy link
Author

MrChuJiu commented Jan 5, 2020

问题在于图像没有与真实图像的链接。
和fabricJS崩溃。我们可以避免崩溃。

Whether such image can be displayed on canvas after solution

@jsprzybylski
Copy link

I'm using fabric 3.6.2 and having similar issue while rendering an image with empty src in node. In my case the problem lays in the way that the element prop is handled:

// fabric.Image.setElement
setElement: function(element, options) {
  ...
  this._element = element; // rendering crashes if element is null 
  this._originalElement = element;
  ...
},
// fabric.Image.getElement
getElement: function() {
  return this._element || {};
},

So the assumption throughout the Image class is that _element prop is an object, but it's not always accessed by getElement method which makes it error prone:

// fabric.Image._renderFill
_renderFill: function(ctx) {
  var elementToDraw = this._element,
         w = this.width, h = this.height,
         sW = Math.min(elementToDraw.naturalWidth || elementToDraw.width, w * this._filterScalingX),
         sH = Math.min(elementToDraw.naturalHeight || elementToDraw.height, h * this._filterScalingY),
  ...
},

The solution would be to either set an empty object if element in setElement is null or access _element through getElement method.

I could probably make an PR but I would need to know how would you like me to handle it.

@asturur
Copy link
Member

asturur commented Apr 21, 2020

Sorry for not getting back to this @jsprzybylski the idea is that we should check in all fabric.Image what happen when an element is missing.

renderFill should skip if no element is available.
toSVG should skip too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants