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

Image is loaded twice #67

Closed
Silex opened this issue Jan 7, 2014 · 7 comments
Closed

Image is loaded twice #67

Silex opened this issue Jan 7, 2014 · 7 comments

Comments

@Silex
Copy link
Contributor

Silex commented Jan 7, 2014

This is visible on index.html when you click Change image and you make your browser show you network requests.

It is because of:

    this.load = function(src, loaded, error) {
        var self = this;

        loaded = loaded || jQuery.noop;
        this._loaded = false;

        //If we assign new image url to the this._img IE9 fires onload event and image width and
        //height are set to zero. So, we create another image object and load image through it.
        var img = new Image();
        img.onload = function() {
            self._loaded = true;
            self._reset(this.width, this.height);

            self._img
                .removeAttr("width")
                .removeAttr("height")
                .removeAttr("style")
                //max-width is reset, because plugin breaks in the twitter bootstrap otherwise
                .css({ position: "absolute", top :"0px", left: "0px", maxWidth: "none"});

            self._img[0].src = src;
            loaded();
        };

        img.onerror = error;

        //we need this because sometimes internet explorer 8 fires onload event
        //right after assignment (synchronously)
        setTimeout(function() {
            img.src = src;
        }, 0);

        this.angle(0);
    };

I understand there are some hacks around IE8/IE9, but I think double loading the image is a serious performance issue. Wouldn't it simpler to drop IE < 11 support? :)

@can3p
Copy link
Collaborator

can3p commented Jan 10, 2014

I don't think that IE9 is old enough to drop it's support but yes multiple requests are the bad thing. One solution is to run this loader only in case of old browsers and use normal loader otherwise.

@mridula-tripathi
Copy link

Hi @can3p, I have very huge images(like >30MB) to display using the plugin. Multiple requests would make my application really slow. And also, since "loaded" is called from inside onload callback for "img", for heavy images, the onFinishLoad is triggered even before the "self._img[0].src" actually loads. Do we have a fix for this yet? In the above code, i tried to invoke "loaded" in onload callback for self._img[0] and it works, but i don think its the right way to fix it. Can you look into the same?

@can3p
Copy link
Collaborator

can3p commented Jun 25, 2014

@mridula-tripathi Are you able to reproduce this bug?

I thought I could do it before, but yesterday I checked the test page in both Google Chrome (Stable, Unstable), Firefox (Nighly) and IE 11 and I don't see any duplicated requests for images.

If you can reproduce the bug, could you please post a step-by-step guide for it?

@Silex
Copy link
Contributor Author

Silex commented Jun 25, 2014

Still happens in Chromium version 34.0.1847.116 Ubuntu 12.04 (260972).

To reproduce just open index.html, and click on the Change Image link at the bottom while looking at requests.

But yeah, maybe this is a chromium bug because it is a fairly old version on ubuntu 12.04.

@can3p
Copy link
Collaborator

can3p commented Jun 25, 2014

I too tend to think that this behaviour is fixed. Could you test on a latest version of Chrome?

can3p added a commit that referenced this issue Jun 27, 2014
@can3p
Copy link
Collaborator

can3p commented Jun 27, 2014

@mridula-tripathi I've accepted your pull request and refactored it. Please, test and tell if the issue is fixed.

@Silex
Copy link
Contributor Author

Silex commented Nov 15, 2014

Yep, looks good now.

@Silex Silex closed this as completed Nov 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants