[PATCH][TEST] Element#getOffsetParent and Element#viewportOffset IE error.

Please test...

Element#viewportOffset PATCH isn't all too well (ref: http://dev.rubyonrails.org/attachment/ticket/11473/viewportOffset_fix.diff)

In fact, new line #594 of that patch anyways sets element to be equal to its element.offsetParent, which we know can be document.documentElement.
Thus in a next pass of the loop, at line #591 (new), the call to element.getOffsetParent() will raise an error: Method or property is missing.
Because this object isn't an extended DOM element (and it shouldn't be)

Wrong :).
The patched getOffsetParent() which the viewportOffset patch is to be used with will not return document.documentElement, but document.body.

The patch is fine.

The element = $(element) issue is addressed in this path: http://dev.rubyonrails.org/ticket/11472[[BR]]

I did not overlap patches to make it easier on the devs.

Fixed link
http://dev.rubyonrails.org/ticket/11472

I'm sorry, jdalton (don't know your name, pardon).

Error occurs on
if (element.getOffsetParent() == document.body &&
line on a a presumably second iteration cycle because element has been previously assigned to by an unsafe assignment in a ending loop statement:
while (element = element.offsetParent); //regular .offsetParent can be null
And thus, in 'if' condition, first time, the 'element' is extended, yes. Yet, the second time it can become a) unextended and b) document.documentElement.
See, in the 'while assignment' line element.offsetParent can be document.documentElement,
and the assignment is done without any checking. (As also stated in a following comment: "regular .offsetParent can be null").
I don't understand why you haven't introduced safe assignment in that line. It seems that that done would fix things.

My data is purely empirical. )

I would recommend the following snippet to replace the current:

var pendingOffset;
do {
	valueT += element.offsetTop	|| 0;
	valueL += element.offsetLeft || 0;

	// Safari fix
	if ((pendingOffset = element.getOffsetParent()) == document.body &&
		Element.getStyle(element, 'position') == 'absolute')) break;

} while (element = pendingOffset);

CORRECTION: Oops, the if statement has doubled parenthesis. Correct version goes like this:

if ((pendingOffset = element.getOffsetParent()) == document.body &&
	Element.getStyle(element, 'position') == 'absolute') break;

CORR.2 (using logical OR):

if ((pendingOffset = element.getOffsetParent()) == document.body ||
	Element.getStyle(element, 'position') == 'absolute') break;

AndrewRev you are right :)
I had tested the getOffsetParent() patch but had eye balled the viewportOffset() patch.
I will correct it soon.

deleted.

deleted

Thanx AdnrewRev,

I have combined the patches for easy comparision of what is changed.
I removed the "Safari fix" because I think it is no longer needed.
Every browser will leave the while loop if the element.getOffsetParent() == document.body
(which is the last thing it can ever be.)

tests for the offsetParent bug

IE with strict doctype may try to return documentElement as offsetParent on relatively positioned elements. This fix correctly returns document.body instead.

Anytime. You're welcome!

fixed element#getOffsetParent() and element#viewportOffset passes unit tests

I believe this addresses ticket: http://dev.rubyonrails.org/ticket/11319 too

Also fixes this ticket: http://dev.rubyonrails.org/ticket/11007

I would recommend that this issue be bumped in severity because it breaks a lot of existing code. The problem (or at least the problem reported to us by several users) is that this behavior became broken only recently on IE 7 because of a recent security update - namely KB 947864:

http://www.microsoft.com/technet/security/Bulletin/MS08-024.mspx

See, e.g.:

http://bugs.horde.org/ticket/6590

If this information is true, and all installations of IE 7 are going to regress, this seems like an awfully severe issue.

Thank you very much for that info, the patch has been pushed to the devs so we will hopefully see it in the core before too long :)

This patch fixed the issue for me, as well. Any reason why it didn't make it into 1.6.0.3?

By the way, I've put up a test page that shows how this bug will throw errors when using Position.clone():

http://jsbin.com/ulebe/

Use http://jsbin.com/ulebe/edit to modify the code, yourself. The two divs are superimposed in FF, but IE throws the error "'style' is null or not an object."