Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#5655 closed bug (fixed)

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@…
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

Change History (8)

comment:1 Changed 10 years ago by vjt

I meant options.of.scrollTop ().

comment:2 Changed 10 years ago by Scott González

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?

comment:3 Changed 10 years ago by vjt

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 :-).

comment:4 Changed 10 years ago by vjt

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. :-)

comment:5 Changed 9 years ago by Jörn Zaefferer

Milestone: TBD1.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.

comment:6 Changed 9 years ago by Scott González

Resolution: fixed
Status: newclosed
Summary: (Possible) Typo in jquery.ui.position 1.8.1Position: document check fails if the scrollTo and document plugins exist

Fixed in 52a052b.

comment:7 Changed 9 years ago by Scott González

Milestone: 1.91.8.5

comment:8 Changed 9 years ago by Scott González

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

Note: See TracTickets for help on using tickets.