Offset: fix wrong origin value #334
Conversation
offset should return undefined instead of {top:0,left:0} for non DOM element.
|
|
|
When will this be merged? |
|
I'd like @dmethvin to do a review as well but I'd like to get it in a Migrate 3.2.0 release that is likely to be available within a week or two. |
|
|
||
| if ( !elem || !elem.nodeType ) { | ||
| migrateWarn( "jQuery.fn.offset() requires a valid DOM element" ); | ||
| return origin; |
There was a problem hiding this comment.
We only want to return undefined for the !elem case. Some code including older older versions of jQuery UI would pass window and expect the top/left of 0. jquery/jquery#3249
There was a problem hiding this comment.
Does it expect {left:0,top:0} for window? Why both 1.12.4 and 2.2.4 return undefined as I tested?
There was a problem hiding this comment.
It sure does, thanks for catching that! I don't have time to look at it right now but I must have been thinking of some other edge case with .offset()? Looks like that ticket is specifically about calling a method on window, not returning the bogus value.
There was a problem hiding this comment.
@dmethvin Since the issue was about a breaking change in jQuery 3.0.0, it seems we shouldn't care about it here as Migrate 3.x is meant to aid with upgrading to the future jQuery 4.0. Am I missing anything?
Fixes #330 .
offsetshould returnundefinedinstead of{top:0,left:0}for non DOM element.