JQMIGRATE: jQuery.fn.removeAttr no longer sets boolean properties: required
https://jquery.com/upgrade-guide/3.0/#breaking-change-removeattr-no-longer-sets-properties-to-false

Stack: OO.ui.TextInputWidget.setRequired / .. / ve.ui.MWReferenceDialog.initialize
When: Opening Reference insert dialog.

Stack (2): OO.ui.TextInputWidget.setRequired / .. / ve.ui.MWSaveDialog.initialize
When: Saving changes.

This does not appear to be a real problem? There is no need to use a property for 'required' (unlike, say, 'checked'). Or am I misunderstanding?

removeAttr( 'required' ) seems to work just fine with jQuery 3, as I would expect. The native removeAttribute() method also works as expected. https://jsfiddle.net/m2x89q34/ https://jsfiddle.net/81w4uok1/

Change 343443 had a related patch set uploaded (by Bartosz Dziewoński):
[oojs/ui] OO.ui.isFocusableElement: Update for jQuery 3 deprecations

https://gerrit.wikimedia.org/r/343443

Change 343443 merged by jenkins-bot:
[oojs/ui] OO.ui.isFocusableElement: Update for jQuery 3 deprecations

https://gerrit.wikimedia.org/r/343443

In T160802#3111945, @matmarex wrote:

JQMIGRATE: jQuery.fn.removeAttr no longer sets boolean properties: required

This does not appear to be a real problem? There is no need to use a property for 'required' (unlike, say, 'checked'). Or am I misunderstanding?

In general, whenever you intend to change the value for a property of an element in JavaScript (as opposed to writing an HTML string in PHP), properties should be used instead of attributes. Valid use cases for changing attributes are essentially limited to these cases:

• Writing a string of HTML.
• Setting attributes on an XML node, or an HTML tree like VisualEditor's data model document, with the intent of serialising the document back to a string. Changing the attribute instead of the property will essentially "save" the value. Since active state (by design) is ignored by HTML/XML serialisation for most properties since documents are expected to roundtrip, the active state, unless explicitly saved to an attribute, or one of the auto-synced properties, should not be reflected to the attributes. The attributes are just a way to see what the initial value was when the browser parsed the HTML document.
• Setting custom data attributes for the purpose of matching CSS attribute selectors.

Unless the semantic intent is one of the above cases, changing the attribute instead of a property is just needless indirection.

Some properties are synced with their attributes, but there's no need to rely on that. Especially given that we couldn't do it consistently because only some properties are synced, and some only synced in one direction.

So you're saying that we should be using .prop() for all attributes that have an associated property? Including e.g. type, title, href, id, but not e.g. role, aria-checked? We never had a written convention for this, but the rule of thumb I was using was .prop() for everything that changes in response to user actions (e.g. value, checked), and .attr() for everything else.

In T160802#3116111, @matmarex wrote:

So you're saying that we should be using .prop() for all attributes that have an associated property? Including e.g. type, title, href, id, but not e.g. role, aria-checked?

Yes. Although for cases where it is trivially synced in both directions it doesn't matter much as it is effectively the same either way (merely a matter of convention/consistency, not of functional difference). E.g. properties with no normalisation (e.g. string values only), that have no form field relation, and can only be changed by JavaScript. (e.g. not changed by user interaction or native code)

type is one I would access through prop() given that it is optional but has a default, and also subject to normalisation. Reading the attribute would return null instead of text if it wasn't explicitly set. Also, setting type=foo as attribute will set the attribute to "foo", but property remains "text", since "foo" is invalid. This is also how feature detection is done for HTML5 input types.

We never had a written convention for this, but the rule of thumb I was using was .prop() for everything that changes in response to user actions (e.g. value, checked), and .attr() for everything else.

A few years ago, I added a brief note about it in the Pitfalls section. Although it is admittedly quite generic and was never enforced (except for a large one-time sprint through all of VisualEditor and OOjs UI at the time).

Change 346468 had a related patch set uploaded (by Bartosz Dziewoński):
[oojs/ui@master] TextInputWidget: Use .prop() rather than .attr() for 'required'

https://gerrit.wikimedia.org/r/346468

Uncaught TypeError: Cannot assign to read only property 'left' of object '#ClientRect'

Looks like in jQuery 3.0.0, $.fn.offset() could directly return the result of HTMLElement#getBoundingClientRect. But in jQuery 3.2.1 (in the current WIP patch), it always returns a plain object. So this should no longer be an issue. But I'm not sure if we can rely on it. Was the original 3.0.0 behavior considered a bug and changed intentionally, or did it just randomly change?

	offset: function( options ) {

		// Preserve chaining for setter
		if ( arguments.length ) {
			return options === undefined ?
				this :
				this.each( function( i ) {
					jQuery.offset.setOffset( this, options, i );
				} );
		}

		var docElem, win, rect, doc,
			elem = this[ 0 ];

		if ( !elem ) {
			return;
		}

		// Support: IE <=11 only
		// Running getBoundingClientRect on a
		// disconnected node in IE throws an error
		if ( !elem.getClientRects().length ) {
			return { top: 0, left: 0 };
		}

		rect = elem.getBoundingClientRect();

		// Make sure element is not hidden (display: none)
		if ( rect.width || rect.height ) {
			doc = elem.ownerDocument;
			win = getWindow( doc );
			docElem = doc.documentElement;

			return {
				top: rect.top + win.pageYOffset - docElem.clientTop,
				left: rect.left + win.pageXOffset - docElem.clientLeft
			};
		}

		// Return zeros for disconnected and hidden elements (gh-2310)
		return rect;
	},

	offset: function( options ) {

		// Preserve chaining for setter
		if ( arguments.length ) {
			return options === undefined ?
				this :
				this.each( function( i ) {
					jQuery.offset.setOffset( this, options, i );
				} );
		}

		var doc, docElem, rect, win,
			elem = this[ 0 ];

		if ( !elem ) {
			return;
		}

		// Return zeros for disconnected and hidden (display: none) elements (gh-2310)
		// Support: IE <=11 only
		// Running getBoundingClientRect on a
		// disconnected node in IE throws an error
		if ( !elem.getClientRects().length ) {
			return { top: 0, left: 0 };
		}

		rect = elem.getBoundingClientRect();

		doc = elem.ownerDocument;
		docElem = doc.documentElement;
		win = doc.defaultView;

		return {
			top: rect.top + win.pageYOffset - docElem.clientTop,
			left: rect.left + win.pageXOffset - docElem.clientLeft
		};
	},

OK, looks like it changed just randomly. https://github.com/jquery/jquery/commit/1777899a747647f3fa839eea4b0bb695d3b60f06

Funnily, it seems like jQuery folks also ran into this issue: https://github.com/jquery/jquery/commit/3befe5911af0cf516896482bb9ddf197c8cb8a8e

Change 346476 had a related patch set uploaded (by Bartosz Dziewoński):
[oojs/ui@master] Make a copy of the result of $.fn.offset() before changing its properties

https://gerrit.wikimedia.org/r/346476

Change 346468 merged by jenkins-bot:
[oojs/ui@master] TextInputWidget: Use .prop() rather than .attr() for 'required'

https://gerrit.wikimedia.org/r/346468

In T160802#3155563, @matmarex wrote:

Uncaught TypeError: Cannot assign to read only property 'left' of object '#ClientRect'

Looks like in jQuery 3.0.0, $.fn.offset() could directly return the result of HTMLElement#getBoundingClientRect. But in jQuery 3.2.1 (in the current WIP patch), it always returns a plain object. So this should no longer be an issue. But I'm not sure if we can rely on it. Was the original 3.0.0 behavior considered a bug and changed intentionally, or did it just randomly change?

Good question. Filed upstream https://github.com/jquery/jquery/issues/3612.

Change 346476 abandoned by Bartosz Dziewoński:
Make a copy of the result of $.fn.offset() before changing its properties

Reason:
No longer needed with the latest versions of jQuery 3.

https://gerrit.wikimedia.org/r/346476

Change 348017 had a related patch set uploaded (by Krinkle; owner: Jforrester):
[oojs/ui@master] Element: Use JSON.parse rather than the deprecated $.parseJSON

https://gerrit.wikimedia.org/r/348017

Change 348017 merged by jenkins-bot:
[oojs/ui@master] Element: Use JSON.parse rather than the deprecated $.parseJSON

https://gerrit.wikimedia.org/r/348017

Change 348036 had a related patch set uploaded (by Jforrester):
[oojs/ui@master] environment: Upgrade jQuery from 1.11.3 to 3.2.1

https://gerrit.wikimedia.org/r/348036

Change 348205 had a related patch set uploaded (by Bartosz Dziewoński):
[oojs/ui@master] tests: Update OO.ui.Process tests for jQuery 3 compatibility

https://gerrit.wikimedia.org/r/348205

Change 348205 merged by jenkins-bot:
[oojs/ui@master] tests: Update OO.ui.Process tests for jQuery 3 compatibility

https://gerrit.wikimedia.org/r/348205

Change 348036 merged by jenkins-bot:
[oojs/ui@master] environment: Upgrade jQuery from 1.11.3 to 3.2.1

https://gerrit.wikimedia.org/r/348036