Skip to content

Fixes jQuery migrate error when getting offset when dropdownParent not in document - #5644

Merged
kevin-brown merged 2 commits into
developfrom
GH-5584
Sep 19, 2019
Merged

Fixes jQuery migrate error when getting offset when dropdownParent not in document#5644
kevin-brown merged 2 commits into
developfrom
GH-5584

Conversation

@kevin-brown

Copy link
Copy Markdown
Member

This pull request includes a

  • Bug fix
  • New feature
  • Translation

The following changes were made

  • AttachBody now checks that dropdownParent is in the document before getting the offset
  • Added a quick test to make sure this consistently happens

If this is related to an existing ticket, include a link to it as well.

Fixes #5584 .

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.
@kevin-brown kevin-brown added this to the 4.0.11 milestone Sep 19, 2019
@kevin-brown
kevin-brown merged commit 7c4131b into develop Sep 19, 2019
@kevin-brown
kevin-brown deleted the GH-5584 branch September 19, 2019 01:56
maxwellfet928 pushed a commit to maxwellfet928/select2 that referenced this pull request Jun 8, 2026
Fixes jQuery migrate error when getting offset when dropdownParent not in document
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant