#5655 closed bug (fixed)

Opened May 25, 2010 04:19PM UTC

Closed August 27, 2010 05:37PM UTC

Last modified November 19, 2010 06:26PM UTC

Position: document check fails if the scrollTo and document plugins exist

Reported by: vjt Owned by:
Priority: minor Milestone: 1.8.5
Component: ui.position Version: 1.8.1
Keywords: scrollTop scrollTo typo Cc: vjt@openssl.it
Blocked by: Blocking:
Description

At line 40 there's the following conditional:

	} else if ( options.of.scrollTo && options.of.document ) {

that was causing elements to be incorrectly positioned when using 

options.of
and the 
jquery.scrollTo
plugin.

To me, looks like a typo, as the conditional should check for 

options.of.scrollTop
:-)

Thanks,

~Marcello

Attachments (0)
Change History (8)

Changed May 25, 2010 04:22PM UTC by vjt comment:1

I meant 

options.of.scrollTop ()
.

Changed May 25, 2010 05:07PM UTC by scottgonzalez comment:2

This is not a typo, we're checking if options.of has the properties scrollTo and document. This is the exact same check that jQuery core uses to determine if an element is the window. Does the scrollTo plugin also add a document property to jQuery objects?

Changed May 25, 2010 07:31PM UTC by vjt comment:3

Heh, no, it's jQuery wysiwyg that defines 

$.fn.document
: unluckily we use it in the same page.

Thus, I think that the proper solution would be to rename plugin methods to something that doesn't clash.

Let me know if and how can I help :-).

Changed May 25, 2010 07:38PM UTC by vjt comment:4

After a quick look to the wysiwyg editor source, I saw that its 

$.fn.document
can be rewritten as a private function, thus without polluting the namespace jQuery. I'll forward the patch to the author and keep you posted. :-)

Changed July 30, 2010 09:53AM UTC by jzaefferer comment:5

milestone: TBD → 1.9

Doesn't look there was a new release of jwysiwyg, so their $.fn.document plugin still exists.

The jQuery core code Scott was referring to seems to be this, its a private helper:

function getWindow( elem ) {
	return ("scrollTo" in elem && elem.document) ?
		elem :
		elem.nodeType === 9 ?
			elem.defaultView || elem.parentWindow :
			false;
}

We could add !options.of.jquery to exclude matching jQuery instances with the right plugin methods.

Changed August 27, 2010 05:37PM UTC by scottgonzalez comment:6

resolution: → fixed
status: new → closed
summary: (Possible) Typo in jquery.ui.position 1.8.1 → Position: document check fails if the scrollTo and document plugins exist

Fixed in 52a052b.

Changed September 10, 2010 05:26PM UTC by scottgonzalez comment:7

milestone: 1.9 → 1.8.5

Changed November 19, 2010 06:26PM UTC by Scott González comment:8

Position: Handle $(document) and $(window) for the of option. Fixes #5963 - Position: option 'of' accepts jQuery object unless it wraps document. Fixes #5655 - (Possible) Typo in jquery.ui.position 1.8.1.

Changeset: 52a052be79d21aa519ccb513dc00a7c54868ef03