Skip to content

Commit

Permalink
Revert "Offset: account for scroll when calculating position"
Browse files Browse the repository at this point in the history
This reverts commit 2d71594.

This commit provoked new issues: jquerygh-2836, jquerygh-2828.

At the meeting, we decided to revert offending commit
(in all three branches - 2.2-stable, 1.12-stable and master)
and tackle this issue in 3.x.

Fixes jquerygh-2828
  • Loading branch information
markelog committed Feb 8, 2016
1 parent 9cb89bf commit 49833f7
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 17 deletions.
7 changes: 2 additions & 5 deletions src/offset.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,8 @@ jQuery.fn.extend( {
}

// Add offsetParent borders
// Subtract offsetParent scroll positions
parentOffset.top += jQuery.css( offsetParent[ 0 ], "borderTopWidth", true ) -
offsetParent.scrollTop();
parentOffset.left += jQuery.css( offsetParent[ 0 ], "borderLeftWidth", true ) -
offsetParent.scrollLeft();
parentOffset.top += jQuery.css( offsetParent[ 0 ], "borderTopWidth", true );
parentOffset.left += jQuery.css( offsetParent[ 0 ], "borderLeftWidth", true );
}

// Subtract parent offsets and element margins
Expand Down
14 changes: 2 additions & 12 deletions test/unit/offset.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ testIframe( "offset/table", "table", function( $, window, document, assert ) {
} );

testIframe( "offset/scroll", "scroll", function( $, win, doc, assert ) {
assert.expect( 30 );
assert.expect( 24 );

assert.equal( $( "#scroll-1" ).offset().top, 7, "jQuery('#scroll-1').offset().top" );
assert.equal( $( "#scroll-1" ).offset().left, 7, "jQuery('#scroll-1').offset().left" );
Expand Down Expand Up @@ -462,17 +462,6 @@ testIframe( "offset/scroll", "scroll", function( $, win, doc, assert ) {
assert.notEqual( $().scrollLeft( null ), null, "jQuery().scrollLeft(null) testing setter on empty jquery object" );
assert.strictEqual( $().scrollTop(), undefined, "jQuery().scrollTop() testing getter on empty jquery object" );
assert.strictEqual( $().scrollLeft(), undefined, "jQuery().scrollLeft() testing getter on empty jquery object" );

// Tests position after parent scrolling (#15239)
$( "#scroll-1" ).scrollTop( 0 );
$( "#scroll-1" ).scrollLeft( 0 );
assert.equal( $( "#scroll-1-1" ).position().top, 6, "jQuery('#scroll-1-1').position().top unaffected by parent scrolling" );
assert.equal( $( "#scroll-1-1" ).position().left, 6, "jQuery('#scroll-1-1').position().left unaffected by parent scrolling" );

$( "#scroll-1" ).scrollTop( 5 );
$( "#scroll-1" ).scrollLeft( 5 );
assert.equal( $( "#scroll-1-1" ).position().top, 6, "jQuery('#scroll-1-1').position().top unaffected by parent scrolling" );
assert.equal( $( "#scroll-1-1" ).position().left, 6, "jQuery('#scroll-1-1').position().left unaffected by parent scrolling" );
} );

testIframe( "offset/body", "body", function( $, window, document, assert ) {
Expand All @@ -486,6 +475,7 @@ testIframe( "offset/body", "body", function( $, window, document, assert ) {

QUnit.test( "chaining", function( assert ) {
assert.expect( 3 );

var coords = { "top": 1, "left": 1 };
assert.equal( jQuery( "#absolute-1" ).offset( coords ).jquery, jQuery.fn.jquery, "offset(coords) returns jQuery object" );
assert.equal( jQuery( "#non-existent" ).offset( coords ).jquery, jQuery.fn.jquery, "offset(coords) with empty jQuery set returns jQuery object" );
Expand Down

0 comments on commit 49833f7

Please sign in to comment.