options.of is used as jQuery object, but it could be a selector string

[T.Lindig]

in the position-plugin the flowing is written:

if ( options.of.nodeType === 9 ) {
	targetWidth = target.width();
	targetHeight = target.height();
	basePosition = { top: 0, left: 0 };
} else if ( options.of.scrollTo && options.of.document ) {
	targetWidth = target.width();
	targetHeight = target.height();
	basePosition = { top: target.scrollTop(), left: target.scrollLeft() };
} else if ( options.of.preventDefault ) {
	// force left top to allow flipping
	options.at = "left top";
	targetWidth = targetHeight = 0;
	basePosition = { top: options.of.pageY, left: options.of.pageX };
} else {
	targetWidth = target.outerWidth();
	targetHeight = target.outerHeight();
	basePosition = target.offset();
}

But options.of could be a string. so options.of.nodeType is always undefined and never === 9.

Instead of options.of you should use the var target. May be so:

if ( target[0].nodeType === 9 ) {
	targetWidth = target.width();
	targetHeight = target.height();
	basePosition = { top: 0, left: 0 };

[Scott González]

Just because options.of can be a string doesn't mean it is. It could be a string, a node, a document, a window, a jQuery object or an event.

[T.Lindig]

bug in position plugin

[T.Lindig]

ok, you misunderstood me.

the problem is, that you use direct options.of instead of the normalized object target.

you access options.of as it would be ever a DOM-object, but that is not save because it could be a string, a node, a document, a window, a jQuery object or an event. (see your answer)

here is the code how i think it should look like:

21 $.fn.position = function( options ) {
22 if ( !options || !options.of ) {
23 return _position.apply( this, arguments );
24 }
25
26 // make a copy, we don't want to modify arguments
27 options = $.extend( {}, options );
28
29 var target = $( options.of ),
30 collision = ( options.collision || "flip" ).split( " " ),
31 offset = options.offset ? options.offset.split( " " ) : [ 0, 0 ],
32 targetWidth,
33 targetHeight,
34 basePosition;
35
36 if ( target[0].nodeType === 9 ) {
37 targetWidth = target.width();
38 targetHeight = target.height();
39 basePosition = { top: 0, left: 0 };
40 } else if ( target[0].scrollTo && target[0].document ) {
41 targetWidth = target.width();
42 targetHeight = target.height();
43 basePosition = { top: target.scrollTop(), left: target.scrollLeft() };
44 } else if ( options.of.preventDefault ) {
45 // force left top to allow flipping
46 options.at = "left top";
47 targetWidth = targetHeight = 0;
48 basePosition = { top: options.of.pageY, left: options.of.pageX };
49 } else {
50 targetWidth = target.outerWidth();
51 targetHeight = target.outerHeight();
52 basePosition = target.offset();
53 } 

In line 44 and 48 I am not sure, how it would be correct. It now works only, if you got a jQuery-Event object because preventDefault, pageY and pageX are properties by jQuery, isn't it? So if you got a regular javascript event-object it will not work.

However, i add also the default_copy.html from the position example and modified the of-option. If run that, you will get exceptions.

[Scott González]

The "normalized object" isn't always valid, as you've obviously noticed since you didn't use it for all of your conditionals. Please stop re-opening this bug.

[T.Lindig]

but it is still a open bug? why you closed it?

[Scott González]

You haven't described a single bug yet.

[T.Lindig]

I have described a bug. I have made and add a test case, where the bug appear. See the attached file line 47. The current implementation did not work with $(document) and $(window) as option.of, but it should.

46	                        $('.positionable').position({
47	                                of: $(document), //$(window) did also not work
48	                                my: $('#my_horizontal').val() + ' ' + $('#my_vertical').val(),
49	                                at: $('#at_horizontal').val() + ' '+ $('#at_vertical').val(),
50	                                offset: $('#offset').val(),
51	                                using: using,
52	                                collision: $("#collision_horizontal").val() + ' ' + $("#collision_vertical").val()

So don't close that ticket before the bug is fixed.

I have described a solution and explained it, this solution works fine and is better, than the current implementation. If you think, it is not the best solution, show us a better one.

[rdworth]

There are two bugs presented here. For clarity I have opened them separately. See:

• #5962 - Position: option 'of' can be other than type selector, undocumented
• #5963 - Position: option 'of' accepts jQuery object unless it wraps document

T.Lindig: In the future, please take care to:

• Present an issue by pointing out a behavior that doesn't match the documented expected behavior. Per #5962, using { of: document } is not documented as supported, but it works. On top of that undocumented feature you were trying a variation, { of: $(document) }. This means what you found is not so much a bug as an undocumented behavior that could be possibly be improved for consistency and convenience.
• Explain clearly what you expected to see and what you saw instead. This is not done by looking at the source code, and certainly not by jumping straight into what line should be changed how. This is done by

1. a simplified test case
2. a list of steps to reproduce
3. the expected behavior (per documention)
4. the actual behavior (the bug you see clearly in the test case)

• Even though you had a test case sooner, steps 2-4 were not done until your very last comment

The current implementation did not work with $(document) and $(window) as option.of, but it should.

• Create an accurate and relevant summary. The summary 'options.of is used as jQuery object, but it could be a selector string' does not describe a present bug because options.of is documented as being a selector string or an event ( see #5962 )
• Keep discussions on the ​Developing jQuery UI Forum. This is a bug tracking system. While it does have a comment capability, it is not designed for back-and-forth meta discourse, and certainly not by way of reopening a ticket again and again in order to further such discussion. Comments and state changes should be related to the summary and description of the ticket. If you read the summary and initial description of this ticket, you'll find that's why it was closed invalid by Scott and Joern. All meta discussion (why was this closed? isn't this a bug? can't options.of sometimes be a string? why don't you use the target? etc.) should occur in a forum appropriate for that.

[Scott González]

Milestone TBD deleted