Skip to content

Commit

Permalink
Fixed an issue with setting offset of absolutely positioned element t…
Browse files Browse the repository at this point in the history
…hat has no position values ("auto"). Fixes #5781.
  • Loading branch information
brandonaaron committed Mar 23, 2010
1 parent 08cf82e commit 656fe92
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 9 deletions.
24 changes: 18 additions & 6 deletions src/offset.js
Expand Up @@ -150,15 +150,27 @@ jQuery.offset = {
},

setOffset: function( elem, options, i ) {
var position = jQuery.curCSS( elem, "position" );

// set position first, in-case top/left are set even on static elem
if ( /static/.test( jQuery.curCSS( elem, "position" ) ) ) {
if ( position === "static" ) {
elem.style.position = "relative";
}
var curElem = jQuery( elem ),
curOffset = curElem.offset(),
curTop = parseInt( jQuery.curCSS( elem, "top", true ), 10 ) || 0,
curLeft = parseInt( jQuery.curCSS( elem, "left", true ), 10 ) || 0,
props = {};

var curElem = jQuery( elem ),
curOffset = curElem.offset(),
curCSSTop = jQuery.curCSS( elem, "top", true ),
curCSSLeft = jQuery.curCSS( elem, "left", true ),
calculatePosition = (position === "absolute" && jQuery.inArray('auto', [curCSSTop, curCSSLeft]) > -1),
props = {}, curPosition = {}, curTop, curLeft;

// need to be able to calculate position if either top or left is auto and position is absolute
if ( calculatePosition ) {
curPosition = curElem.position();
}

curTop = calculatePosition ? curPosition.top : parseInt( curCSSTop, 10 ) || 0;
curLeft = calculatePosition ? curPosition.left : parseInt( curCSSLeft, 10 ) || 0;

if ( jQuery.isFunction( options ) ) {
options = options.call( elem, i, curOffset );
Expand Down
2 changes: 2 additions & 0 deletions test/data/offset/absolute.html
Expand Up @@ -13,6 +13,7 @@
#absolute-2 { top: 19px; left: 19px; }
#marker { position: absolute; border: 2px solid #000; width: 50px; height: 50px; background: #ccc; }
p.instructions { position: absolute; bottom: 0; }
#positionTest { position: absolute; }
</style>
<script type="text/javascript" src="../../../dist/jquery.js"></script>
<script type="text/javascript" charset="utf-8">
Expand All @@ -33,6 +34,7 @@
</div>
</div>
<div id="absolute-2" class="absolute">absolute-2</div>
<div id="positionTest">Has absolute position but no values set for the location ('auto').</div>
<div id="marker"></div>
<p class="instructions">Click the white box to move the marker to it. Clicking the box also changes the position to absolute (if not already) and sets the position according to the position method.</p>
</body>
Expand Down
12 changes: 9 additions & 3 deletions test/unit/offset.js
Expand Up @@ -35,7 +35,7 @@ testoffset("absolute"/* in iframe */, function($, iframe) {
});

testoffset("absolute", function( jQuery ) {
expect(176);
expect(178);

// get offset tests
var tests = [
Expand All @@ -62,6 +62,11 @@ testoffset("absolute", function( jQuery ) {
equals( jQuery( this.id ).position().left, this.left, "jQuery('" + this.id + "').position().left" );
});

// test #5781
var offset = jQuery( '#positionTest' ).offset({ top: 10, left: 10 }).offset();
equals( offset.top, 10, "Setting offset on element with position absolute but 'auto' values." )
equals( offset.left, 10, "Setting offset on element with position absolute but 'auto' values." )


// set offset
tests = [
Expand Down Expand Up @@ -97,8 +102,9 @@ testoffset("absolute", function( jQuery ) {
equals( jQuery( this.id ).offset().top, this.top + 1, "jQuery('" + this.id + "').offset({ top: " + (this.top + 1) + " })" );
equals( jQuery( this.id ).offset().left, this.left + 1, "jQuery('" + this.id + "').offset({ left: " + (this.left + 1) + " })" );

jQuery( this.id ).offset({ top: this.top + 2 });
jQuery( this.id ).offset({ left: this.left + 2 });
jQuery( this.id )
.offset({ left: this.left + 2 })
.offset({ top: this.top + 2 });
equals( jQuery( this.id ).offset().top, this.top + 2, "Setting one property at a time." );
equals( jQuery( this.id ).offset().left, this.left + 2, "Setting one property at a time." );

Expand Down

1 comment on commit 656fe92

@jzaefferer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this. I can't even count how often I had to put .css({top:0, left:0}) in front of my calls to .position({my: ...}).

Please sign in to comment.