Skip to content

/ jquery

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

Jump to bottom

`.position()` and `.offsetParent()` return incorrect value #3107

Closed
anseki opened this issue May 5, 2016 · 2 comments
Closed

`.position()` and `.offsetParent()` return incorrect value #3107

anseki opened this issue May 5, 2016 · 2 comments
Assignees
@gibson042
Labels
Has Pull Request Offset
Milestone
3.3.0

Comments

@anseki
Copy link

@anseki anseki commented May 5, 2016

.position() method should return a position of element relative to a closest ancestor element that is positioned.
Therefore, for example, if CSS properties top/left of the element that has position:absolute are set to that returned position, the element should not move.
But when the ancestor element that is positioned does not exist (i.e. all ancestor elements have position:static, this is default), a position relative to <html> element incorrectly is returned.
For example:
https://jsfiddle.net/fqbgy5pq/

Result:

ClientRect   top: 63   left: 63
CSS props    top: auto left: auto
.position()  top: 59   left: 59
Set return value of `.position()` to `top` and `left`, then the element should not move.
ClientRect   top: 59   left: 59
CSS props    top: 59px left: 59px
.position()  top: 55   left: 55

Note: 2px was lost by this issue, and more 2px was lost by another issue of .offset().
When fixed .offset() is used:

Result:

ClientRect   top: 63   left: 63
CSS props    top: auto left: auto
.position()  top: 61   left: 61
Set return value of `.position()` to `top` and `left`, then the element should not move.
ClientRect   top: 61   left: 61
CSS props    top: 61px left: 61px
.position()  top: 59   left: 59

.offsetParent() method that is called by .position() method returns <html> when ancestor element that is positioned is not found. Then, .position() mistakes.

jquery/src/offset.js

Line 180 in 305f193

return offsetParent || documentElement;

I think that .offsetParent() should return document or null in this case.
document means "base of coordinates".
null means "ancestor element is not found".

Also, .position() method should return coordinates relative to the document in this case.

And also, .position() method returns incorrect coordinates when <html> has position:non-static (i.e. the ancestor element is really <html>).
For example:
https://jsfiddle.net/wczL6ae5/

Result:

ClientRect   top: 63   left: 63
CSS props    top: auto left: auto
.position()  top: 59   left: 59
Set return value of `.position()` to `top` and `left`, then the element should not move.
ClientRect   top: 62   left: 62
CSS props    top: 59px left: 59px
.position()  top: 58   left: 58

Note: 1px was added by this issue, and 2px was lost by another issue of .offset().
When fixed .offset() is used:

Result:

ClientRect   top: 63   left: 63
CSS props    top: auto left: auto
.position()  top: 61   left: 61
Set return value of `.position()` to `top` and `left`, then the element should not move.
ClientRect   top: 64   left: 64
CSS props    top: 61px left: 61px
.position()  top: 62   left: 62

When .offsetParent() method returned <html>, .position() method ignores its offset.

jquery/src/offset.js

Line 144 in 305f193

if ( !jQuery.nodeName( offsetParent[ 0 ], "html" ) ) {

Therefore, if the <html> has offset (i.e. margin), returned value is incorrect.
Strange to say, only border-width is included in that calculation.

jquery/src/offset.js

Line 150 in 305f193

top: parentOffset.top + jQuery.css( offsetParent[ 0 ], "borderTopWidth", true ),

I think that it might affect scripts that exist if the return value of .offsetParent() method is changed.
At least, .position() should be fixed even if .offsetParent() is not fixed. In this case, .position() should not use .offsetParent(), or it should check whether position is static when <html> was returned.
This issue affects .offset() method because setter of .offset() uses .position().
@markelog
Copy link
Member

@markelog markelog commented May 16, 2016 •
edited

I think that .offsetParent() should return document or null in this case.
document means "base of coordinates".
null means "ancestor element is not found".

This behaviour is already defined in spec - https://drafts.csswg.org/cssom-view/#dom-htmlelement-offsetparent, we deviate from it though -

jquery/src/offset.js

Lines 162 to 166 in 95c7ab6

// This method will return documentElement in the following cases:
// 1) For the element inside the iframe without offsetParent, this method will return
// documentElement of the parent window
// 2) For the hidden or detached element
// 3) For body or html element, i.e. in case of the html node - it will return itself

There is a lot of issues with offsetParent - #1765 :/
@markelog markelog added the Offset label May 16, 2016
@anseki
Copy link
Author

@anseki anseki commented May 17, 2016

If jQuery doesn't behave as with CSSOM specification, I think that .position() should be fixed.
@gibson042 gibson042 mentioned this issue Jun 6, 2016
Closed
3 of 4 tasks complete
@timmywil timmywil modified the milestone: 3.1.1 Jun 30, 2016
@timmywil timmywil modified the milestones: 3.1.1, 3.2.0 Jul 13, 2016
gibson042 added a commit to gibson042/jquery that referenced this issue Jan 5, 2017
@gibson042
Offset: Add .position() tests for unusual html/body element styles
bea5371 
Ref jquerygh-3107
gibson042 added a commit to gibson042/jquery that referenced this issue Jan 5, 2017
@gibson042
Offset: Identify the correct offset parent in .position()
5f3eb78 
Fixes jquerygh-3107
@gibson042 gibson042 mentioned this issue Jan 5, 2017
Merged
4 of 4 tasks complete
@timmywil timmywil modified the milestones: 3.2.0, 3.3.0 Mar 15, 2017
gibson042 added a commit to gibson042/jquery that referenced this issue Mar 20, 2017
@gibson042
Offset: Add .position() tests for unusual html/body element styles
6c440a4 
Ref jquerygh-3107
gibson042 added a commit to gibson042/jquery that referenced this issue Mar 20, 2017
@gibson042
Offset: Identify the correct offset parent in .position()
986f8c3 
Fixes jquerygh-3107
gibson042 added a commit that referenced this issue Apr 24, 2017
@gibson042
Offset: Use correct offset parents; include all border/scroll values
Loading status checks…
1d2df77 
Thanks @anseki

Fixes gh-3080
Fixes gh-3107
Closes gh-3096
Closes gh-3487
@jlukic jlukic mentioned this issue Dec 12, 2017
Closed
@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.
Assignees

@gibson042 gibson042

Labels
Has Pull Request Offset
Projects
None yet
Milestone
3.3.0
Linked pull requests

Successfully merging a pull request may close this issue.

Fix .offset() and .position() bugs
4 participants
@timmywil @markelog @gibson042 @anseki