#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: | [email protected]… |
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 13 years ago by
comment:2 Changed 13 years ago by
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 13 years ago by
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 13 years ago by
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 13 years ago by
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.
comment:6 Changed 12 years ago by
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.
comment:7 Changed 12 years ago by
Milestone: | 1.9 → 1.8.5 |
---|
comment:8 Changed 12 years ago by
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
I meant
options.of.scrollTop ()
.