Skip to content

Commit c4bb90c

Browse files
committed
Offset: Fix the offset patch
Changes: * `.offset()` works on `document.documentElement` * `.offset()` setter on an empty object returns the source object, not undefined * `.offset()` setter on a disconnected node returns the source object, not `{ top: 0, left: 0}`
1 parent 4b1f6ab commit c4bb90c

File tree

2 files changed

+42
-7
lines changed

2 files changed

+42
-7
lines changed

src/jquery/offset.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,26 @@ import { migrateWarn } from "../main.js";
22

33
var oldOffset = jQuery.fn.offset;
44

5+
function isAttached( elem ) {
6+
return jQuery.contains( elem.ownerDocument, elem );
7+
}
8+
59
jQuery.fn.offset = function() {
6-
var docElem,
7-
elem = this[ 0 ],
10+
var elem = this[ 0 ],
811
bogus = { top: 0, left: 0 };
912

1013
if ( !elem || !elem.nodeType ) {
1114
migrateWarn( "jQuery.fn.offset() requires a valid DOM element" );
12-
return undefined;
15+
return arguments.length ? this : undefined;
1316
}
1417

15-
docElem = ( elem.ownerDocument || window.document ).documentElement;
16-
if ( !jQuery.contains( docElem, elem ) ) {
18+
if ( !isAttached( elem ) ) {
1719
migrateWarn( "jQuery.fn.offset() requires an element connected to a document" );
18-
return bogus;
20+
21+
// Only return the bogus value for the getter.
22+
if ( !arguments.length ) {
23+
return bogus;
24+
}
1925
}
2026

2127
return oldOffset.apply( this, arguments );

test/offset.js

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
QUnit.module( "offset" );
33

44
QUnit.test( ".offset()", function( assert ) {
5-
assert.expect( 12 );
5+
assert.expect( 20 );
66

77
var bogus = { top: 0, left: 0 };
88

@@ -38,10 +38,39 @@ QUnit.test( ".offset()", function( assert ) {
3838
);
3939
} );
4040

41+
expectWarning( assert, ".offset() as setter on disconnected node", 2,
42+
function() {
43+
var $elemInitial = jQuery( "<div />" )
44+
.css( "position", "fixed" ),
45+
$elem = $elemInitial
46+
.offset( { top: 42, left: 99 } );
47+
48+
assert.strictEqual( $elem[ 0 ], $elemInitial[ 0 ],
49+
".offset() returns a proper jQuery object" );
50+
51+
$elem.appendTo( "#qunit-fixture" );
52+
assert.deepEqual( $elem.offset(), { top: 42, left: 99 } );
53+
} );
54+
55+
expectWarning( assert, ".offset() on empty set", 2, function() {
56+
var $empty = jQuery();
57+
58+
assert.strictEqual( $empty.offset(), undefined, ".offset() returns undefined" );
59+
assert.strictEqual( $empty.offset( { top: 42, left: 99 } ), $empty,
60+
".offset( coords ) returns the jQuery object" );
61+
} );
62+
4163
expectWarning( assert, ".offset() on plain object", function() {
4264
assert.strictEqual(
4365
jQuery( { space: "junk", zero: 0 } ).offset(),
4466
undefined, "plain object undefined"
4567
);
4668
} );
69+
70+
expectNoWarning( assert, ".offset() on documentElement", function() {
71+
assert.deepEqual(
72+
jQuery( document.documentElement ).offset(),
73+
{ top: 0, left: 0 }, "no crash"
74+
);
75+
} );
4776
} );

0 commit comments

Comments
 (0)