Description

Working on deprecating isWindow, I ran into this internal function in the offset module:

/**
 * Gets a window from an element
 */
function getWindow( elem ) {
	return jQuery.isWindow( elem ) ? elem : elem.nodeType === 9 && elem.defaultView;
}

The comment is incorrect. It returns a reference to window if passed a reference to a window or document node, but it returns false if passed a reference to an element node.

As a side note, it seems very odd to try to derive a window reference at all, considering that many methods simply reference the window that is passed to the core during initialization.

For example:

// Miscellaneous
"target": function( elem ) {
	var hash = window.location && window.location.hash;
	return hash && hash.slice( 1 ) === elem.id;
}

Or perhaps that's just a bug. Do see that some methods at least attempt to work with multiple windows. But I digress.

The inappropriately named getWindow function is used in two places. As always, if we can reduce that number to one, we can get rid of the function (and the costs associated with calling it).

First one is in the offset method:

doc = elem.ownerDocument;
win = getWindow( doc );

So that obviously reduces to:

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

The other is here:

// Create scrollLeft and scrollTop methods
jQuery.each( { scrollLeft: "pageXOffset", scrollTop: "pageYOffset" }, function( method, prop ) {
	var top = "pageYOffset" === prop;

	jQuery.fn[ method ] = function( val ) {
		return access( this, function( elem, method, val ) {
			var win = getWindow( elem );

			if ( val === undefined ) {
				return win ? win[ prop ] : elem[ method ];
			}

			if ( win ) {
				win.scrollTo(
					!top ? val : win.pageXOffset,
					top ? val : win.pageYOffset
				);

			} else {
				elem[ method ] = val;
			}
		}, method, val, arguments.length );
	};
} );

That confirms the function is expected to return a "falsey" value when passed an element reference. Something like this should replace the getWindow call:

var win;
			
if ( elem.window === elem ) {
	win = elem;
} else if ( elem.nodeType == 9 ) {
	win = elem.defaultView;
}

...and then that function can be deleted.

One less isWindow call in the bargain as well. As that one is public, it will have to be deprecated rather than deleted. That's next.

HTH

PS. Consider that a large number of issues are related to the ill-advised design of the object model, which tries to deal with elements, documents and windows (among other things) using a single constructor. It's an awkward interface for users as well. Was a similar mistake with attr, which tried to deal with both (HTML) properties and (XML) attributes using a single function.

Link to test case

No test case needed. Simply restructuring.

I agree with the proposed changes to eliminate getWindow. Pull request welcome.

Not sure how to do that with my fork in its current state. Would have to create another fork I expect. This change came about while eliminating the bigger issue of isWindow and there are a dozen other changes in my fork at this point.

Might be easier for somebody else to fix this based on my suggestions. As was the case with this one (which was closed due prematurely for same reason):

#3444