Skip to content

Commit 44b2180

Browse files
committed
Draggable: normalize lookups for rootNodes when to bust scroll cache. Fixes #9379 - Draggable: position bug in scrollable div
Core: update scrollParent() to support all current supported browsers.
1 parent 77bf202 commit 44b2180

File tree

5 files changed

+85
-53
lines changed

5 files changed

+85
-53
lines changed

tests/unit/draggable/draggable.html

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
top: 0;
2020
}
2121
#main-forceScrollable {
22+
position: absolute;
23+
left: 0;
24+
top: 0;
2225
width: 1100px;
2326
height: 1100px;
2427
}
@@ -33,6 +36,9 @@
3336
overflow-y: hidden;
3437
}
3538
#scrollParent-forceScrollable {
39+
position: absolute;
40+
left: 0;
41+
top: 0;
3642
width: 1300px;
3743
height: 1300px;
3844
}
@@ -78,7 +84,7 @@ <h2 id="qunit-userAgent"></h2>
7884
<div id="qunit-fixture">
7985
<div id="scrollParent">
8086
<div id="main">
81-
<div id="draggable1" style="background: green; width: 200px; height: 100px;">Relative</div>
87+
<div id="draggable1" style="background: green; width: 200px; height: 100px; position: relative; top: 0; left: 0;">Relative</div>
8288
<div id="draggable2" style="background: green; width: 200px; height: 100px; position: absolute; top: 10px; left: 10px;"><span><em>Absolute</em></span></div>
8389
<div id="droppable" style="background: green; width: 200px; height: 100px; position: absolute; top: 110px; left: 110px;"><span>Absolute</span></div>
8490
<div id="main-forceScrollable"></div>

tests/unit/draggable/draggable_core.js

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ test( "#6258: not following mouse when scrolled and using overflow-y: scroll", f
136136
});
137137
});
138138

139-
test( "#9315: Draggable: jumps down with offset of scrollbar", function() {
139+
test( "#9315: jumps down with offset of scrollbar", function() {
140140
expect( 2 );
141141

142142
var element = $( "#draggable2" ).draggable({
@@ -186,6 +186,41 @@ test( "#5009: scroll not working with parent's position fixed", function() {
186186
});
187187
});
188188

189+
test( "#9379: Draggable: position bug in scrollable div", function() {
190+
expect( 2 );
191+
192+
$( "#qunit-fixture" ).html( "<div id='o_9379'><div id='i_9379'></div><div id='d_9379'>a</div></div>" );
193+
$( "#i_9379" ).css({ position: "absolute", width: "500px", height: "500px" });
194+
$( "#o_9379" ).css({ position: "absolute", width: "300px", height: "300px" });
195+
$( "#d_9379" ).css({ width: "10px", height: "10px" });
196+
197+
var moves = 3,
198+
startValue = 0,
199+
dragDelta = 20,
200+
delta = 100,
201+
202+
// we scroll after each drag event, so subtract 1 from number of moves for expected
203+
expected = delta + ( ( moves - 1 ) * dragDelta ),
204+
element = $( "#d_9379" ).draggable({
205+
drag: function() {
206+
startValue += dragDelta;
207+
$( "#o_9379" ).scrollTop( startValue ).scrollLeft( startValue );
208+
},
209+
stop: function( event, ui ) {
210+
equal( ui.position.left, expected, "left position is correct when grandparent is scrolled" );
211+
equal( ui.position.top, expected, "top position is correct when grandparent is scrolled" );
212+
}
213+
});
214+
215+
$( "#o_9379" ).css( "overflow", "auto" );
216+
217+
element.simulate( "drag", {
218+
dy: delta,
219+
dx: delta,
220+
moves: moves
221+
});
222+
});
223+
189224
test( "#5727: draggable from iframe" , function() {
190225
expect( 1 );
191226

tests/unit/draggable/draggable_options.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ test( "containment, account for border", function() {
377377
test( "containment, default, switching after initialization", function() {
378378
expect( 6 );
379379

380-
var element = $( "#draggable1" ).draggable({ containment: false });
380+
var element = $( "#draggable1" ).draggable({ containment: false, scroll: false });
381381

382382
TestHelpers.draggable.testDrag( element, element, -100, -100, -100, -100, "containment: default" );
383383

@@ -692,7 +692,7 @@ test( "helper, default, switching after initialization", function() {
692692
scroll: false
693693
});
694694

695-
if ( scrollElements.length === 1 && scrollElements[ 1 ] === "#scrollParent" ) {
695+
if ( scrollElements.length === 1 && scrollElements[ 0 ] === "#scrollParent" ) {
696696
TestHelpers.draggable.setScrollable( "#main", false );
697697
TestHelpers.draggable.setScrollable( "#scrollParent", true );
698698
}
@@ -867,6 +867,8 @@ test( "scroll, scrollSensitivity, and scrollSpeed", function() {
867867
test( "#6817: auto scroll goes double distance when dragging", function() {
868868
expect( 2 );
869869

870+
TestHelpers.draggable.restoreScroll( document );
871+
870872
var offsetBefore,
871873
distance = 10,
872874
viewportHeight = $( window ).height(),
@@ -906,6 +908,7 @@ test( "snap, snapMode, and snapTolerance", function() {
906908
snapTolerance = 15,
907909
element = $( "#draggable1" ).draggable({
908910
snap: true,
911+
scroll: false,
909912
snapMode: "both",
910913
snapTolerance: snapTolerance
911914
}),
@@ -1028,6 +1031,7 @@ test( "#8459: element can snap to an element that was removed during drag", func
10281031
snapTolerance = 15,
10291032
element = $( "#draggable1" ).draggable({
10301033
snap: true,
1034+
scroll: false,
10311035
snapMode: "both",
10321036
snapTolerance: snapTolerance,
10331037
start: function() {

ui/jquery.ui.core.js

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,17 @@ $.fn.extend({
5555
})( $.fn.focus ),
5656

5757
scrollParent: function() {
58-
var scrollParent;
59-
if (($.ui.ie && (/(static|relative)/).test(this.css("position"))) || (/absolute/).test(this.css("position"))) {
60-
scrollParent = this.parents().filter(function() {
61-
return (/(relative|absolute|fixed)/).test($.css(this,"position")) && (/(auto|scroll)/).test($.css(this,"overflow")+$.css(this,"overflow-y")+$.css(this,"overflow-x"));
62-
}).eq(0);
63-
} else {
64-
scrollParent = this.parents().filter(function() {
65-
return (/(auto|scroll)/).test($.css(this,"overflow")+$.css(this,"overflow-y")+$.css(this,"overflow-x"));
66-
}).eq(0);
67-
}
58+
var position = this.css( "position" ),
59+
excludeStaticParent = position === "absolute",
60+
scrollParent = this.parents().filter( function() {
61+
var parent = $( this );
62+
if ( excludeStaticParent && parent.css( "position" ) === "static" ) {
63+
return false;
64+
}
65+
return (/(auto|scroll)/).test( parent.css( "overflow" ) + parent.css( "overflow-y" ) + parent.css( "overflow-x" ) );
66+
}).eq( 0 );
6867

69-
return ( /fixed/ ).test( this.css( "position") ) || !scrollParent.length ? $( this[ 0 ].ownerDocument || document ) : scrollParent;
68+
return position === "fixed" || !scrollParent.length ? $( this[ 0 ].ownerDocument || document ) : scrollParent;
7069
},
7170

7271
uniqueId: (function() {

ui/jquery.ui.draggable.js

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,10 @@ $.widget("ui.draggable", $.ui.mouse, {
334334
}
335335
},
336336

337+
_isRootNode: function( element ) {
338+
return ( /(html|body)/i ).test( element.tagName ) || element === this.document[ 0 ];
339+
},
340+
337341
_getParentOffset: function() {
338342

339343
//Get the offsetParent and cache its position
@@ -349,10 +353,7 @@ $.widget("ui.draggable", $.ui.mouse, {
349353
po.top += this.scrollParent.scrollTop();
350354
}
351355

352-
//This needs to be actually done for all browsers, since pageX/pageY includes this information
353-
//Ugly IE fix
354-
if((this.offsetParent[0] === document.body) ||
355-
(this.offsetParent[0].tagName && this.offsetParent[0].tagName.toLowerCase() === "html" && $.ui.ie)) {
356+
if ( this._isRootNode( this.offsetParent[ 0 ] ) ) {
356357
po = { top: 0, left: 0 };
357358
}
358359

@@ -364,17 +365,18 @@ $.widget("ui.draggable", $.ui.mouse, {
364365
},
365366

366367
_getRelativeOffset: function() {
367-
368-
if(this.cssPosition === "relative") {
369-
var p = this.element.position();
370-
return {
371-
top: p.top - (parseInt(this.helper.css("top"),10) || 0) + this.scrollParent.scrollTop(),
372-
left: p.left - (parseInt(this.helper.css("left"),10) || 0) + this.scrollParent.scrollLeft()
373-
};
374-
} else {
368+
if ( this.cssPosition !== "relative" ) {
375369
return { top: 0, left: 0 };
376370
}
377371

372+
var p = this.element.position(),
373+
scrollIsRootNode = this._isRootNode( this.scrollParent[ 0 ] );
374+
375+
return {
376+
top: p.top - ( parseInt(this.helper.css( "top" ), 10) || 0 ) + ( !scrollIsRootNode ? this.scrollParent.scrollTop() : 0 ),
377+
left: p.left - ( parseInt(this.helper.css( "left" ), 10) || 0 ) + ( !scrollIsRootNode ? this.scrollParent.scrollLeft() : 0 )
378+
};
379+
378380
},
379381

380382
_cacheMargins: function() {
@@ -458,31 +460,20 @@ $.widget("ui.draggable", $.ui.mouse, {
458460
}
459461

460462
var mod = d === "absolute" ? 1 : -1,
461-
document = this.document[ 0 ],
462-
useOffsetParent = this.cssPosition === "absolute" && ( this.scrollParent[ 0 ] === document || !$.contains( this.scrollParent[ 0 ], this.offsetParent[ 0 ] ) ),
463-
scroll = useOffsetParent ? this.offsetParent : this.scrollParent,
464-
// we need to test if offsetParent was used here because Blink incorrectly reports a 0 scrollTop
465-
// on document.documentElement when the page is scrolled. Checking for offsetParent normalizes
466-
// this across browsers. Blink bug: https://code.google.com/p/chromium/issues/detail?id=157855
467-
scrollIsRootNode = useOffsetParent && ( /(html|body)/i ).test( scroll[ 0 ].nodeName );
468-
469-
//Cache the scroll
470-
if (!this.offset.scroll) {
471-
this.offset.scroll = {top : scroll.scrollTop(), left : scroll.scrollLeft()};
472-
}
463+
scrollIsRootNode = this._isRootNode( this.scrollParent[ 0 ] );
473464

474465
return {
475466
top: (
476467
pos.top + // The absolute mouse position
477468
this.offset.relative.top * mod + // Only for relative positioned nodes: Relative offset from element to offset parent
478469
this.offset.parent.top * mod - // The offsetParent's offset without borders (offset + border)
479-
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) ) * mod)
470+
( ( this.cssPosition === "fixed" ? -this.offset.scroll.top : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) ) * mod)
480471
),
481472
left: (
482473
pos.left + // The absolute mouse position
483474
this.offset.relative.left * mod + // Only for relative positioned nodes: Relative offset from element to offset parent
484475
this.offset.parent.left * mod - // The offsetParent's offset without borders (offset + border)
485-
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : this.offset.scroll.left ) * mod)
476+
( ( this.cssPosition === "fixed" ? -this.offset.scroll.left : ( scrollIsRootNode ? 0 : this.offset.scroll.left ) ) * mod)
486477
)
487478
};
488479

@@ -492,19 +483,16 @@ $.widget("ui.draggable", $.ui.mouse, {
492483

493484
var containment, co, top, left,
494485
o = this.options,
495-
document = this.document[ 0 ],
496-
useOffsetParent = this.cssPosition === "absolute" && ( this.scrollParent[ 0 ] === document || !$.contains( this.scrollParent[ 0 ], this.offsetParent[ 0 ] ) ),
497-
scroll = useOffsetParent ? this.offsetParent : this.scrollParent,
498-
// we need to test if offsetParent was used here because Blink incorrectly reports a 0 scrollTop
499-
// on document.documentElement when the page is scrolled. Checking for offsetParent normalizes
500-
// this across browsers. Blink bug: https://code.google.com/p/chromium/issues/detail?id=157855
501-
scrollIsRootNode = useOffsetParent && ( /(html|body)/i ).test( scroll[ 0 ].nodeName ),
486+
scrollIsRootNode = this._isRootNode( this.scrollParent[ 0 ] ),
502487
pageX = event.pageX,
503488
pageY = event.pageY;
504489

505-
//Cache the scroll
506-
if (!this.offset.scroll) {
507-
this.offset.scroll = {top : scroll.scrollTop(), left : scroll.scrollLeft()};
490+
// Cache the scroll
491+
if ( !scrollIsRootNode || !this.offset.scroll ) {
492+
this.offset.scroll = {
493+
top: this.scrollParent.scrollTop(),
494+
left: this.scrollParent.scrollLeft()
495+
};
508496
}
509497

510498
/*
@@ -566,14 +554,14 @@ $.widget("ui.draggable", $.ui.mouse, {
566554
this.offset.click.top - // Click offset (relative to the element)
567555
this.offset.relative.top - // Only for relative positioned nodes: Relative offset from element to offset parent
568556
this.offset.parent.top + // The offsetParent's offset without borders (offset + border)
569-
( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) )
557+
( this.cssPosition === "fixed" ? -this.offset.scroll.top : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) )
570558
),
571559
left: (
572560
pageX - // The absolute mouse position
573561
this.offset.click.left - // Click offset (relative to the element)
574562
this.offset.relative.left - // Only for relative positioned nodes: Relative offset from element to offset parent
575563
this.offset.parent.left + // The offsetParent's offset without borders (offset + border)
576-
( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : ( scrollIsRootNode ? 0 : this.offset.scroll.left ) )
564+
( this.cssPosition === "fixed" ? -this.offset.scroll.left : ( scrollIsRootNode ? 0 : this.offset.scroll.left ) )
577565
)
578566
};
579567

0 commit comments

Comments
 (0)