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

Progress doesn't fire after .then() for resolved promises #2013

Closed
homm opened this Issue Jan 14, 2015 · 2 comments

Comments

Projects
None yet
3 participants
@homm

homm commented Jan 14, 2015

Suppose we have the Deferred object and try to set handlers on progress for both .promise() and .then() objects:

var d = $.Deferred().notify();
d.promise().progress(function(){ console.log('promise') });
d.then().progress(function(){ console.log('then') });

This works as expected, both progress handlers will fire. But if we try to set progress handlers for resolved Deferred, the result will be different:

var d = $.Deferred().notify().resolve(); // Look there, I'm resolved.
d.promise().progress(function(){ console.log('promise') });
d.then().progress(function(){ console.log('then') });

Only first handler will fire. The problem in tuples order. Progress tuple come last, and therefor, when newDefer.notify() is called, newDefer is already resolved and doesn't accept notifying.

This bug is similar to #1894, but slightly different.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Jan 14, 2015

Member

I think this a dup of #2010 but I'll let @gibson042 make the final call.

I would say that this may not work in the near future with .then if we make it strictly Promise/A compatible, and you'd need to use Deferred-specific methods. Again, that hasn't been decided but as best practice it would be good to expect standard behavior from standard names.

Member

dmethvin commented Jan 14, 2015

I think this a dup of #2010 but I'll let @gibson042 make the final call.

I would say that this may not work in the near future with .then if we make it strictly Promise/A compatible, and you'd need to use Deferred-specific methods. Again, that hasn't been decided but as best practice it would be good to expect standard behavior from standard names.

homm added a commit to uploadcare/uploadcare-widget that referenced this issue Jan 14, 2015

@gibson042

This comment has been minimized.

Show comment
Hide comment
@gibson042

gibson042 Jan 14, 2015

Member

Extremely similar, but not quite a duplicate... @homm, would you like to submit a PR? Both the code and new tests should be straightforward.

Member

gibson042 commented Jan 14, 2015

Extremely similar, but not quite a duplicate... @homm, would you like to submit a PR? Both the code and new tests should be straightforward.

@dmethvin dmethvin added the Deferred label Jan 15, 2015

gibson042 added a commit to gibson042/jquery that referenced this issue Apr 14, 2015

@gibson042 gibson042 self-assigned this Apr 14, 2015

gibson042 added a commit that referenced this issue Apr 22, 2015

@gibson042 gibson042 closed this in 002240a Apr 22, 2015

homm added a commit to uploadcare/uploadcare-widget that referenced this issue May 15, 2015

fix jQuery bug jquery/jquery#2013
Progress was not shown in some situaations with crop

@cssmagic cssmagic referenced this issue May 18, 2016

Open

jQuery #5

@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.