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

load() callback "this" breaking in 2.2.0 vs 2.1.4 #3035

Closed
blq opened this Issue Apr 4, 2016 · 7 comments

Comments

Projects
None yet
5 participants
@blq

blq commented Apr 4, 2016

Hi,
"this" in the callback to the load() function changed from 2.1.4 to 2.2.0 in a breaking way.

The documentation at http://api.jquery.com/load/ says: ".. this is set to each DOM element in turn.".
In 2.2.0 "this" is instead the jQuery instance, and not the DOM element.

I tracked it down to this commit:
a4715f4

Regards
/Fredrik Blomqvist

@gibson042

This comment has been minimized.

Show comment
Hide comment
@gibson042

gibson042 Apr 4, 2016

Member

It looks like the callback invocation should be using this instead of self. Would you like to open a pull request?

Member

gibson042 commented Apr 4, 2016

It looks like the callback invocation should be using this instead of self. Would you like to open a pull request?

@gibson042 gibson042 added the Ajax label Apr 4, 2016

@mgol mgol added this to the 1.12.3/2.2.3 milestone Apr 4, 2016

@mgol mgol added the Bug label Apr 4, 2016

@timmywil timmywil self-assigned this Apr 4, 2016

@markelog markelog modified the milestones: 3.0.0, 1.12.3/2.2.3 Apr 4, 2016

markelog added a commit to markelog/jquery that referenced this issue Apr 4, 2016

markelog added a commit to markelog/jquery that referenced this issue Apr 4, 2016

@markelog markelog assigned markelog and unassigned timmywil Apr 4, 2016

@markelog markelog added the Blocker label Apr 4, 2016

@timmywil timmywil closed this in 5d20a3c Apr 4, 2016

timmywil added a commit that referenced this issue Apr 4, 2016

timmywil added a commit that referenced this issue Apr 4, 2016

@mgol mgol modified the milestones: 1.12.3/2.2.3, 3.0.0 Apr 4, 2016

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Apr 4, 2016

Member

@markelog This fix is making it into 1.12.3 & 2.2.3 so the milestone should reflect that. Why did you change that to 3.0.0?

Member

mgol commented Apr 4, 2016

@markelog This fix is making it into 1.12.3 & 2.2.3 so the milestone should reflect that. Why did you change that to 3.0.0?

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Apr 4, 2016

Member

And to 3.0 too, whereas master branch is a primary one. So why did you change the milestone, before clarifying?

Member

markelog commented Apr 4, 2016

And to 3.0 too, whereas master branch is a primary one. So why did you change the milestone, before clarifying?

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Apr 4, 2016

Member

Milestones are used to generate changelogs and for end users it doesn't
matter that 3.0.0 got the fix separately, for them it's fixed in
1.12.3/2.2.3 and just stays fixed in 3.0.0. That's why the older releases
get the milestone and that's what's we've been doing so far.

Also, if anyone, I should be the one asking why you changed the milestone
without claryfing.

Michał Gołębiowski

Member

mgol commented Apr 4, 2016

Milestones are used to generate changelogs and for end users it doesn't
matter that 3.0.0 got the fix separately, for them it's fixed in
1.12.3/2.2.3 and just stays fixed in 3.0.0. That's why the older releases
get the milestone and that's what's we've been doing so far.

Also, if anyone, I should be the one asking why you changed the milestone
without claryfing.

Michał Gołębiowski

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Apr 4, 2016

Member

I think correct course of events is important, this issue looks like that the fix is only needed for 2.2-stable, whereas is it would incorrect to assume that.

for them it's fixed in 1.12.3/2.2.3 and just stays fixed in 3.0.0

That would be an incorrect, offensive commit was done against the 3.0 and ported to 2.x/1.x,
not to mentioned to we already released several versions of 3.x, pre-releases but releases nonetheless, which has this bug, now you propose to omit this commit and thereby deprive users from this information? All and all i don't understand why you propose to mislead our users, since i assumed you set the milestone by mistake and not on purpose, that is why i have changed the milestone.

This commit should be in both changelogs, milestone, in my opinion should be set on primary branch.

Member

markelog commented Apr 4, 2016

I think correct course of events is important, this issue looks like that the fix is only needed for 2.2-stable, whereas is it would incorrect to assume that.

for them it's fixed in 1.12.3/2.2.3 and just stays fixed in 3.0.0

That would be an incorrect, offensive commit was done against the 3.0 and ported to 2.x/1.x,
not to mentioned to we already released several versions of 3.x, pre-releases but releases nonetheless, which has this bug, now you propose to omit this commit and thereby deprive users from this information? All and all i don't understand why you propose to mislead our users, since i assumed you set the milestone by mistake and not on purpose, that is why i have changed the milestone.

This commit should be in both changelogs, milestone, in my opinion should be set on primary branch.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Apr 4, 2016

Member

I disagree but I don't have anything else to add.

Member

mgol commented Apr 4, 2016

I disagree but I don't have anything else to add.

@blq

This comment has been minimized.

Show comment
Hide comment
@blq

blq Apr 6, 2016

Thanks for the fix and quick release, now our app works again! :)

blq commented Apr 6, 2016

Thanks for the fix and quick release, now our app works again! :)

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

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