Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Offset Module: Get Rid of getWindow Function #3449
Comments
|
I agree with the proposed changes to eliminate |
|
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 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): |
Description
Working on deprecating
isWindow, I ran into this internal function in the offset module:The comment is incorrect. It returns a reference to
windowif passed a reference to awindowordocumentnode, but it returnsfalseif passed a reference to an element node.As a side note, it seems very odd to try to derive a
windowreference at all, considering that many methods simply reference thewindowthat is passed to the core during initialization.For example:
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
getWindowfunction 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
offsetmethod:So that obviously reduces to:
The other is here:
That confirms the function is expected to return a "falsey" value when passed an element reference. Something like this should replace the
getWindowcall:...and then that function can be deleted.
One less
isWindowcall 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.