-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Support Relative URL for Image src #5484
Conversation
…a to work with dynamic sized canvas.
…dims are not available. Add test.
… for image in broswer.
@@ -349,7 +357,13 @@ | |||
if (element.toDataURL) { | |||
return element.toDataURL(); | |||
} | |||
return element.src; | |||
|
|||
if (this.srcFromAttribute && typeof element.getAttribute === 'function') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We support ie10 ( sort of ) ie11 and then modern browsers.
Can you verify if getAttribute is really unsupported from some of those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (this.srcFromAttribute && typeof element.getAttribute === 'function') { | |
if (this.srcFromAttribute) { |
@nateevans a couple of comments:
Can you try to add a test in the relative file in the test/unit folder? |
dist/fabric.js
Outdated
* When calling {@link fabric.Image.getSrc}, return value from element src with `element.getAttribute('src')` (if available). | ||
* This allows for relative urls as image src. | ||
* @type Boolean | ||
* @default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add:
@since 2.7.0 ?
@nateevans a couple of comments: branch off your master from where you are, restore your master to the top of fabric.js master, and before committing the code remember to do: or find the commit where dist was not changed and do |
i wonder how we can properly test node. as it is now the test under node does not make much sense. |
Ah ok. How do you run tests normally? seems like I had to build those in order to run
I don't think the use case for this actually applies to node. With JSDOM in Node, calling |
I'll close out this PR and make one from the new branch |
see #5476