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

Implemented fix written by Bartosz Grzesiak for this problem: #214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

@marcenuc
Copy link

I can reproduce this bug on many computers (Windows and Ubuntu) with different versions of Firefox (9, 10, and 11beta). This patch fixes the problem.

Is there a reason why it isn't merged? I can try to find a better solution if I know why this fix is not applied.

@marcenuc
Copy link

OK, I've verified that this patch doesn't fix the problem for IE, because "load needs to be added before the src attribute". The correct line is:

$( '<img/>' ).load( function() { complete.call( this ) } ).attr( 'src', $( img ).attr( 'src' ) );

This works for me on IE8, Firefox10, and Chrome on Windows and Linux.

@danimt
Copy link

danimt commented Mar 12, 2012

Thanks for the fix marcenuc. Working with IE9 aswell ;)

@marcenuc
Copy link

Seems like it also works on IE7/8, as reported on #245.

@davidhellsing
Copy link
Contributor

I would consider pulling this in, but in all honesty this "fix" seems like a random extra load event attached to a new IMG element. No docs or discussion about what the code actually does, why it "works" or in what use cases it needs to be applied. I’m not going to add an extra load event just because it "works" in some cases, since the original code works fine for 99% of the users.

That said, I am all open for improving the code. Start by attaching a dev site where the bug occurs, and some tests that indicates why it happens and we’ll take it from there.

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

Successfully merging this pull request may close these issues.

4 participants