-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Fix IE rendering of SVG as overlay image #4324
Conversation
Could you provide a failing fiddle to run with ie11/ie10? i m sort of ok with the fix, i would just make it a member of fabric.util Also i would target it just at SVGs and not for each dataurl. |
@asturur Here you go: You might have to refresh a few times (the Pre-fix version), because it is random (timing?) I'll refactor my code a bit as you suggested and update the PR. |
… 'loadImageInDom'
✅ Refactor to |
What is the difference between appending on the onload event, and make something more slow peaced like, detect if is an dataurl svg, append to dom, then set the source, and then in the onload event ( and in the onerror ) remove from dom ? And probably would require another way to remove it from dom, maybe returning the div node reference from the method that appends it to body. I have do not time now to test everything ( node included ) so take your time to think of it, i m completely ok with this fix. |
@asturur The 1ms is enough, since IE somehow only requires the DOM element to be present and visible (outside of the viewport = fine). I tried the cleanup without the I don't know exactly why, but this the (a) fix in IE11. There may be a cleaner approach (for instance: without a 👍 |
P.S. This was the problem in IE too; this PR fixes it :) #2945 (comment) |
i mean something: loadImage: function(url, callback, context, crossOrigin) {
if (!url) {
callback && callback.call(context, url);
return;
}
var img = fabric.util.createImage();
var domNode;
if (url.substring(0,14) === 'data:image/svg') {
// IE10 / IE11-Fix: SVG contents from data: URI
// will only be available if the IMG is present
// in the DOM (and visible)
domNode = fabric.util.loadImageInDom(img);
} /** @ignore */
img.onload = function () {
if (domNode) {
// remove node from dom
}
callback && callback.call(context, img);
img = img.onload = img.onerror = null;
}; Sort of letting the image in the dom before the load process start, seems more natural to me. And could give enough time to render it, unless for ie11 the load is just pull the string in memory without render. |
Nice one. I will test it. |
@asturur Tried it, doesn't work :( If the callback is called after creating the DOM element, IE11 still breaks. If the callback happens after a 1ms delay , IE's fine with it 🙄 However, I did move some logic around, this is a bit cleaner; all DOM related stuff is in |
i guess if avoid setting the img to null ( pointless since GC would take care of it anyway, could solve the 1ms issue. |
@asturur Tried it. Doesn't work :( |
To me the code looks exactly what i wanted, plus some extra function wrapping that are not a problem/blocker. |
IE11 (and sometimes IE10, timing issue) won't "in memory render" SVG images when the source is a data:... URI, if the img-element isn't present and visible in the DOM. Even when appending the IMG to the DOM with
display: none
or something like that, the img will be 0x0 px.This ugly-ass fix will create a div at -1px -1px (absolute) and append the img in this div. After this, de callback will be able to get the image contents.
You can reproduce the problem in IE11 by using a data:... URI to use a SVG for
fabric.Image.fromURL
(to use it as an overlay withcanvas.setOverlayImage
).With this fix, this works fine :)