Skip to content
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

jQuery v3.6.0 breaks Bootstrap's test suite in IE #4859

Closed
XhmikosR opened this issue Mar 15, 2021 · 20 comments · Fixed by #5223
Closed

jQuery v3.6.0 breaks Bootstrap's test suite in IE #4859

XhmikosR opened this issue Mar 15, 2021 · 20 comments · Fixed by #5223
Assignees
Milestone

Comments

@XhmikosR
Copy link

XhmikosR commented Mar 15, 2021

Opening the issue after quickly chatting with mgol on IRC

I made a branch to update jQuery to v3.6.0 in twbs/bootstrap#33243. I notice our BrowserStack tests fail consistently on Internet Explorer 10 and 11. v3.5.1 works fine.

git clone https://github.com/twbs/bootstrap.git -b v4-dev-xmr-update-jquery
npm i
npm run js-test-cloud #assumes you have BrowserStack credentials set up

The failures seem to point to https://github.com/twbs/bootstrap/blob/v4-dev/js/tests/unit/dropdown.js#L860 and https://github.com/twbs/bootstrap/blob/v4-dev/js/tests/unit/dropdown.js#L870

EDIT: using jQuery v3.3.1 also works fine.

@mgol mgol added Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. Event labels Mar 15, 2021
@mgol mgol changed the title v3.6.0 Internet Explorer breaks our test suite jQuery v3.6.0 breaks Bootstrap's test suite in IE Mar 15, 2021
@mgol mgol added the Bug label Mar 15, 2021
@mgol
Copy link
Member

mgol commented Mar 15, 2021

An isolated test case is basicaly:

input1.trigger('focus');
input2.trigger('focus');
input1.trigger('focus');

In IE, focus stays on input2.

See versions with jQuery:

The 3.6.0 version fails in IE, the older ones succeed.

jQuery 3.4 should be similar to 3.5 as there weren't major changes in relevant areas.

I think this is the same bug that makes some tests fail in jQuery UI only with jQuery 3.6.0+:

Also related to #4856, although that issue manifests itself in all 3.4+ versions.

@timmywil timmywil added this to the 3.6.1 milestone Mar 15, 2021
@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Mar 15, 2021
@timmywil
Copy link
Member

We'll address this in a patch. Related: dbcffb3

@gibson042
Copy link
Member

gibson042 commented Mar 15, 2021

@mgol The idea discussed in today's weekly meeting:

 // Ensure the presence of an event listener that handles manually-triggered
 // synthetic events by interrupting progress until reinvoked in response to
 // *native* events that it fires directly, ensuring that state changes have
 // already occurred before other listeners are invoked.
 function leverageNative( el, type, expectSync ) {
+	var result = {}, queue = [];
 
 	// Missing expectSync indicates a trigger call, which must force setup through jQuery.event.add
 	if ( !expectSync ) {
 		if ( dataPriv.get( el, type ) === undefined ) {
 			jQuery.event.add( el, type, returnTrue );
 		}
 		return;
 	}
 
 	// Register the controller as a special universal handler for all event namespaces
 	dataPriv.set( el, type, false );
 	jQuery.event.add( el, type, {
 		namespace: false,
 		handler: function( event ) {
-			var notAsync, result,
-				saved = dataPriv.get( this, type );
+			var forceSync,
+				i = 0,
+				length = queue.length,
+				oldResult = result;
 
 			if ( ( event.isTrigger & 1 ) && this[ type ] ) {
 
 				// Interrupt processing of the outer synthetic .trigger()ed event
-				// Saved data should be false in such cases, but might be a leftover capture object
-				// from an async native handler (gh-4350)
-				if ( !saved.length ) {
+				if ( !event.isInner ) {
 
 					// Store arguments for use when handling the inner native event
-					// There will always be at least one argument (an event object), so this array
-					// will not be confused with a leftover capture object.
-					saved = slice.call( arguments );
-					dataPriv.set( this, type, saved );
+					queue.push( slice.call( arguments ) );
 
 					// Trigger the native event and capture its result
 					// Support: IE <=9 - 11+
 					// focus() and blur() are asynchronous
-					notAsync = expectSync( this, type );
+					forceSync = expectSync( this, type );
 					this[ type ]();
-					result = dataPriv.get( this, type );
-					if ( saved !== result || notAsync ) {
-						dataPriv.set( this, type, false );
-					} else {
-						result = {};
-					}
-					if ( saved !== result ) {
+					if ( result === oldResult && !forceSync ) {
+						result = {};
+					}
+					if ( result !== oldResult ) {
 
 						// Cancel the outer synthetic event
 						event.stopImmediatePropagation();
 						event.preventDefault();
 
 						// Support: Chrome 86+
 						// In Chrome, if an element having a focusout handler is blurred by
 						// clicking outside of it, it invokes the handler synchronously. If
 						// that handler calls `.remove()` on the element, the data is cleared,
 						// leaving `result` undefined. We need to guard against this.
 						return result && result.value;
 					}
 
 				// If this is an inner synthetic event for an event with a bubbling surrogate
 				// (focus or blur), assume that the surrogate already propagated from triggering the
 				// native event and prevent that from happening again here.
 				// This technically gets the ordering wrong w.r.t. to `.trigger()` (in which the
 				// bubbling surrogate propagates *after* the non-bubbling base), but that seems
 				// less bad than duplication.
 				} else if ( ( jQuery.event.special[ type ] || {} ).delegateType ) {
 					event.stopPropagation();
 				}
 
 			// If this is a native event triggered above, everything is now in order
 			// Fire an inner synthetic event with the original arguments
+			// for each queued event, and capture the last result
-			} else if ( saved.length ) {
+			} else if ( length ) {
 
-				// ...and capture the result
-				dataPriv.set( this, type, {
+				for ( ; i < length; i++ ) {
+				result = ( {
 					value: jQuery.event.trigger(
 
 						// Support: IE <=9 - 11+
 						// Extend with the prototype to reset the above stopImmediatePropagation()
-						jQuery.extend( saved[ 0 ], jQuery.Event.prototype ),
-						saved.slice( 1 ),
+						jQuery.extend( queue[ 0 ][ 0 ], jQuery.Event.prototype, { isInner: true } ),
+						queue.shift().slice( 1 ),
 						this
 					)
 				} );
+				}
 
 				// Abort handling of the native event
 				event.stopImmediatePropagation();
 			}
 		}
 	} );
 }

The concept should be sufficient to invoke handlers as expected, even with code like input1.trigger('focus', 0); input2.trigger('focus'); input1.trigger('focus', 1).

@mgol
Copy link
Member

mgol commented Mar 15, 2021

@gibson042 Thanks! Do you want to put that in a PR or just to provide a starting point for others to pick up?

@jimmywarting

This comment has been minimized.

@mgol

This comment has been minimized.

@jimmywarting

This comment has been minimized.

@mgol

This comment has been minimized.

@ghost
Copy link

ghost commented Jul 7, 2021

oddly we are experiencing similar issue in chrome, where's user has to doubleclick in order to refocus on elements with content editable. We had to move from 3.4.1 to 3.6

@mgol
Copy link
Member

mgol commented Jul 7, 2021

@Novolokov do I understand it right that you didn't have the issue in 3.4.1 but you have one on 3.6.0? Any chance for a test case? If you have one, please open a separate issue.

@ghost
Copy link

ghost commented Jul 8, 2021

@mgol, sorry too difficult to isolate. used this to fix it https://stackoverflow.com/a/17384592

jwmerrill added a commit to desmosinc/mathquill that referenced this issue Nov 12, 2021
jQuery 3.x has introduced a bunch of different focus bugs in the process
of attempting to migrate to using native focus events in more cases,
some of which remain unfixed in the latest version (3.6.0).

Examples:
* jquery/jquery#4859
* jquery/jquery#4856
* jquery/jquery#4950
* jquery/jquery#4867

Some unknown bug in this general class can make it so that Mathquill
public api focus calls stop working because using jQuery to
programatically trigger focus breaks.

Work around this by switching to triggering native focus events instead
of jQuery focus events.
jwmerrill added a commit to desmosinc/mathquill that referenced this issue Nov 16, 2021
jQuery 3.x has introduced a bunch of different focus bugs in the process
of attempting to migrate to using native focus events in more cases,
some of which remain unfixed in the latest version (3.6.0).

Examples:
* jquery/jquery#4859
* jquery/jquery#4856
* jquery/jquery#4950
* jquery/jquery#4867

Some unknown bug in this general class can make it so that Mathquill
public api focus calls stop working because using jQuery to
programatically trigger focus breaks.

Work around this by switching to triggering native focus events instead
of jQuery focus events.
gibson042 added a commit to gibson042/jquery that referenced this issue Jan 24, 2022
@timmywil timmywil modified the milestones: 3.6.1, 3.7.0 Aug 15, 2022
mgol added a commit to mgol/jquery that referenced this issue Mar 13, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronouslu. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Also, simplify `leverageNative` - With IE now using `focusin` to simulate
`focus` and `focusout` to simulate `blur`, we don't have to deal with
async events in `leverageNative`.

Fixes jquerygh-4856
Fixes jquerygh-4859
mgol added a commit to mgol/jquery that referenced this issue Mar 13, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronouslu. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
mgol added a commit to mgol/jquery that referenced this issue Mar 14, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronouslu. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
@mgol
Copy link
Member

mgol commented Mar 14, 2023

It looks like we can fix this via simulating the async focus & blur in IE via their bubbling but also synchronous equivalents: focusin & focusout. In other browsers, all those events are synchronous.

PRs for:

@mgol mgol self-assigned this Mar 14, 2023
mgol added a commit to mgol/jquery that referenced this issue Mar 14, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronouslu. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
mgol added a commit to mgol/jquery that referenced this issue Mar 14, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronouslu. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
mgol added a commit to mgol/jquery that referenced this issue Mar 14, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronouslu. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
mgol added a commit to mgol/jquery that referenced this issue Mar 14, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
mgol added a commit to mgol/jquery that referenced this issue Mar 14, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
mgol added a commit to mgol/jquery that referenced this issue Mar 15, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
mgol added a commit to mgol/jquery that referenced this issue Mar 15, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
mgol added a commit to mgol/jquery that referenced this issue Mar 17, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
Fixes jquerygh-4950
mgol added a commit to mgol/jquery that referenced this issue Mar 17, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
Fixes jquerygh-4950
mgol added a commit to mgol/jquery that referenced this issue Mar 20, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
Fixes jquerygh-4950
mgol added a commit to mgol/jquery that referenced this issue Mar 20, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously.

Simulate `focus` via `focusin` & `blur` via `focusout` in IE to
avoid these issues.

Fixes jquerygh-4856
Fixes jquerygh-4859
Fixes jquerygh-4950
mgol added a commit that referenced this issue Mar 27, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously. Asynchronous behavior of these
handlers in IE caused issues for IE (gh-4856, gh-4859).

We now simulate `focus` via `focusin` & `blur` via `focusout` in IE to avoid
these issues. This also let us simplify some tests.

This commit also simplifies `leverageNative` - with IE now using `focusin`
to simulate `focus` and `focusout` to simulate `blur`, we don't have to deal
with async events in `leverageNative`. This also fixes broken `focus` triggers
after first triggering it on a hidden element - previously, `leverageNative`
assumed that the native `focus` handler not firing after calling the native 
`focus` method meant it would be handled later, asynchronously, which
was not the case (gh-4950).

Fixes gh-4856
Fixes gh-4859
Fixes gh-4950
Closes gh-5223

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
mgol added a commit that referenced this issue Mar 27, 2023
In IE (all versions), `focus` & `blur` handlers are fired asynchronously
but `focusin` & `focusout` are run synchronously. In other browsers, all
those handlers are fired synchronously. Asynchronous behavior of these
handlers in IE caused issues for IE (gh-4856, gh-4859).

We now simulate `focus` via `focusin` & `blur` via `focusout` in IE to avoid
these issues. This also let us simplify some tests.

This commit also simplifies `leverageNative` - with IE now using `focusin`
to simulate `focus` and `focusout` to simulate `blur`, we don't have to deal
with async events in `leverageNative`. This also fixes broken `focus` triggers
after first triggering it on a hidden element - previously, `leverageNative`
assumed that the native `focus` handler not firing after calling the native 
`focus` method meant it would be handled later, asynchronously, which
was not the case (gh-4950).

To preserve relative `focusin`/`focus` & `focusout`/`blur` event order
guaranteed on the 3.x branch, attach a single handler for both events in IE.

A side effect of this is that to reduce size the `event/focusin` module
no longer exists and it's impossible to disable the `focusin` patch
in modern browsers via the jQuery custom build system.

Fixes gh-4856
Fixes gh-4859
Fixes gh-4950
Ref gh-5223
Closes gh-5224

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
@mgol
Copy link
Member

mgol commented Mar 27, 2023

Fixed on main (future 4.x) via #5223 and on 3.x-stable via #5224.

@mgol
Copy link
Member

mgol commented Apr 4, 2023

@XhmikosR could you check whether https://releases.jquery.com/git/jquery-3.x-git.js works for you? This should be more or less what we'll release as jQuery 3.7.0.

@XhmikosR
Copy link
Author

XhmikosR commented Apr 4, 2023 via email

@mgol
Copy link
Member

mgol commented Apr 4, 2023

I understand, but my hope was to check before we release. 😉 Our release process is quite complex, so we don’t do a lot of pre-releases for minor updates.

FWIW, I did test manually using steps you provided above with jQuery swapped for the 3.x-git version and they passed. Is there anything else to test?

@XhmikosR
Copy link
Author

XhmikosR commented Apr 4, 2023

If you tried the following and works on BrowserStack then it should be fine:

git clone https://github.com/twbs/bootstrap.git -b v4-dev
npm i
# change the jquery version to the one you want to test
npm run js-test-cloud #assumes you have BrowserStack credentials set up

@mgol
Copy link
Member

mgol commented Apr 4, 2023

Yes, that’s what I did. It looks like we’re good then!

@XhmikosR
Copy link
Author

XhmikosR commented Apr 4, 2023

Perfect, thanks for fixing the issue!

@mgol
Copy link
Member

mgol commented May 12, 2023

@XhmikosR jQuery 3.7.0 has been released, you should be able to update it in Bootstrap v4 now.

https://blog.jquery.com/2023/05/11/jquery-3-7-0-released-staying-in-order/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment