Skip to content

Tests: Account for extra focus/blur listeners in jQuery >=3.4 #1930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 23, 2020

Conversation

mgol
Copy link
Member

@mgol mgol commented Jun 24, 2020

jQuery >=3.4.0 uses a special focus/blur handler pair
needed to fix various issues with checkboxes/radio buttons
as well as being able to pass data in focus triggers.
However, this leaves dummy focus & blur events if any of these
events were ever listened to at a particular element. There's not
a lot UI can do to fix this so we now just skip these handlers for
data comparisons in tests.

Ref jquery/jquery#4496

jQuery >=3.4.0 uses a special focus/blur handler pair
needed to fix various issues with checkboxes/radio buttons
as well as being able to pass data in focus triggers.
However, this leaves dummy focus & blur events if any of these
events were ever listened to at a particular element. There's not
a lot UI can do to fix this so we now just skip these handlers for
data comparisons in tests.

Ref jquery/jquery#4496
@mgol mgol added this to the 1.13 milestone Jun 24, 2020
@mgol mgol requested review from fnagel and arschmitz June 24, 2020 16:35
Copy link
Member

@fnagel fnagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 by reading

Copy link
Member

@timmywil timmywil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. A little hacky, but should be future safe.

Copy link
Member

@gibson042 gibson042 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix fails to filter out the dummy handlers added when .trigger precedes .on, but that would only be a problem if such cases exist in the test suite. Apparently they don't, so this works! 👍

// a lot UI can do to fix this so just skip these handlers for
// data comparisons in tests.
// See https://github.com/jquery/jquery/issues/4496
if ( result.events && jQueryVersionSince( "3.4.0" ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need to restrict this code to jQuery >= 3.4.0? What would happen without the check?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought since this excludes some things from the compared objects, in older jQueries where there are no additional listeners we could perform a full check. There's always a risk that the workaround will not account for some real differences and by restricting it to jQuery >=3.4.0 we'd see a test error in older jQuery versions in such a case.

UI is currently tested with Core >=1.8.

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
@mgol mgol merged commit 5e2fc44 into jquery:master Jul 23, 2020
@mgol mgol deleted the tests-destroy-jquery3.4-workaround branch July 23, 2020 13:54
mgol added a commit to mgol/jquery-ui that referenced this pull request Feb 19, 2021
jQuery >=3.4.0 uses a special focus/blur handler pair needed to fix various
issues with checkboxes/radio buttons as well as being able to pass data in focus
triggers. This leaves extra focus & blur events if any of these events were ever
listened to at a particular element.

We've started skipping these handlers in the `domEqual` assertion in jquerygh-1930 but
we missed a case where an event is triggered before any handler is attached -
jQuery >=3.4.0 attaches then an extra noop listener just to force the code path
to go through the setup code before the trigger happens. We now skip this extra
handler as well.

This fixes a test failure in "dialog: methods" destroy tests.

Ref jquery/jquery#4496
Ref jquerygh-1930
mgol added a commit to mgol/jquery-ui that referenced this pull request Feb 19, 2021
jQuery >=3.4.0 uses a special focus/blur handler pair needed to fix various
issues with checkboxes/radio buttons as well as being able to pass data in focus
triggers. This leaves extra focus & blur events if any of these events were ever
listened to at a particular element.

We've started skipping these handlers in the `domEqual` assertion in jquerygh-1930 but
we missed a case where an event is triggered before any handler is attached -
jQuery >=3.4.0 attaches then an extra noop listener just to force the code path
to go through the setup code before the trigger happens. We now skip this extra
handler as well.

This fixes a test failure in "dialog: methods" destroy tests.

Ref jquery/jquery#4496
Ref jquerygh-1930
mgol added a commit to mgol/jquery-ui that referenced this pull request Feb 19, 2021
jQuery >=3.4.0 uses a special focus/blur handler pair needed to fix various
issues with checkboxes/radio buttons as well as being able to pass data in focus
triggers. This leaves extra focus & blur events if any of these events were ever
listened to at a particular element.

We've started skipping these handlers in the `domEqual` assertion in jquerygh-1930 but
we missed a case where an event is triggered before any handler is attached -
jQuery >=3.4.0 attaches then an extra noop listener just to force the code path
to go through the setup code before the trigger happens. We now skip this extra
handler as well.

This fixes a test failure in "dialog: methods" destroy tests.

Ref jquery/jquery#4496
Ref jquerygh-1930
mgol added a commit that referenced this pull request Feb 20, 2021
jQuery >=3.4.0 uses a special focus/blur handler pair needed to fix various
issues with checkboxes/radio buttons as well as being able to pass data in focus
triggers. This leaves extra focus & blur events if any of these events were ever
listened to at a particular element.

We've started skipping these handlers in the `domEqual` assertion in gh-1930 but
we missed a case where an event is triggered before any handler is attached -
jQuery >=3.4.0 attaches then an extra noop listener just to force the code path
to go through the setup code before the trigger happens. We now skip this extra
handler as well.

This fixes a test failure in "dialog: methods" destroy tests.

Closes gh-1945
Ref jquery/jquery#4496
Ref gh-1930
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants