Skip to content

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 arschmitz and fnagel 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

6 participants