From 2c5467b38cfac17aa87ff4ac09370e61d8bf4053 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Wed, 18 Sep 2019 21:33:13 -0400 Subject: [PATCH 1/2] Fixes error when getting offset of element not in document This fixes an error which is called out in jQuery Migrate but probably never happens in real life. This is because we call `jQuery.fn.offset` without checking if the element is in the document. Based on testing done here and within the MediaWiki team, I'm pretty sure jQuery never actually implemented explicit checks and this jQuery Migrate warning is just to cover the case where a browser might start returning inconsistnet results. And we could at least reproduce the inconsistency, so that's something. We now default the offset to 0/0 if the parent element happens to not be in the document. This appears to be what jQuery used to do in the past, and generally appears to be what people expect in these cases. This fixes #5584. --- src/js/select2/dropdown/attachBody.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/js/select2/dropdown/attachBody.js b/src/js/select2/dropdown/attachBody.js index 7380d9c6..1ef2373c 100644 --- a/src/js/select2/dropdown/attachBody.js +++ b/src/js/select2/dropdown/attachBody.js @@ -190,7 +190,14 @@ define([ $offsetParent = $offsetParent.offsetParent(); } - var parentOffset = $offsetParent.offset(); + var parentOffset = { + top: 0, + left: 0 + }; + + if ($.contains(document.body, $offsetParent[0])) { + parentOffset = $offsetParent.offset(); + } css.top -= parentOffset.top; css.left -= parentOffset.left; From 31931a4212226c350d9db0a5906886bbdd47b19c Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Wed, 18 Sep 2019 21:44:25 -0400 Subject: [PATCH 2/2] Add tests for when dropdownParent isn't in the document --- tests/dropdown/positioning-tests.js | 51 ++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/tests/dropdown/positioning-tests.js b/tests/dropdown/positioning-tests.js index 2da68982..eb721654 100644 --- a/tests/dropdown/positioning-tests.js +++ b/tests/dropdown/positioning-tests.js @@ -174,4 +174,53 @@ test('dropdown is positioned down with absolute offsets', function (assert) { '0px', 'There should not be an extra left offset' ); -}); \ No newline at end of file +}); + +test('dropdown is positioned even when not in document', function (assert) { + var $ = require('jquery'); + var $select = $(''); + + var $container = $('test'); + var container = new MockContainer(); + + var Utils = require('select2/utils'); + var Options = require('select2/options'); + + var Dropdown = require('select2/dropdown'); + var AttachBody = require('select2/dropdown/attachBody'); + + var DropdownAdapter = Utils.Decorate(Dropdown, AttachBody); + + var dropdown = new DropdownAdapter($select, new Options({ + dropdownParent: $('html') + })); + + var $dropdown = dropdown.render(); + + assert.equal( + $dropdown[0].style.top, + 0, + 'The drodpown should not have any offset before it is displayed' + ); + + dropdown.bind(container, $container); + dropdown.position($dropdown, $container); + dropdown._showDropdown(); + + assert.ok( + dropdown.$dropdown.hasClass('select2-dropdown--below'), + 'The dropdown should be forced down' + ); + + assert.equal( + $dropdown.css('top'), + '0px', + 'The offset should be 0px at the top' + ); + + assert.equal( + $dropdown.css('left'), + '0px', + 'The offset should be 0px on the left' + ); +});