Skip to main content

Bug Tracker

Side navigation

#4310 closed enhancement (wontfix)

Opened March 08, 2009 01:33PM UTC

Closed April 21, 2009 07:24PM UTC

jQuery.fn.offset - simplify and remove "parseInt"

Reported by: roviury Owned by: brandon
Priority: major Milestone: 1.4
Component: offset Version: 1.3.2
Keywords: Cc:
Blocked by: Blocking:
Description

jQuery.fn.offset can be more simplify.

parseInt is not necessary.

it can change to...

jQuery.fn.offset = function() {
	var elem = this[0];
  if ( !elem ) return { top: 0, left: 0 };
  var doc = elem.ownerDocument, body = doc.body;
	if ( elem === body ) return jQuery.offset.bodyOffset( elem );
	if ( document.documentElement["getBoundingClientRect"] ){
		var box  = elem.getBoundingClientRect(), docElem = doc.documentElement,
			clientTop = docElem.clientTop || body.clientTop || 0, clientLeft = docElem.clientLeft || body.clientLeft || 0,
			top  = box.top  + (self.pageYOffset || jQuery.boxModel && docElem.scrollTop  || body.scrollTop ) - clientTop,
			left = box.left + (self.pageXOffset || jQuery.boxModel && docElem.scrollLeft || body.scrollLeft) - clientLeft;
	}else{
		jQuery.offset.initialized || jQuery.offset.initialize();

		var offsetParent = elem.offsetParent, prevOffsetParent = elem,
			doc = elem.ownerDocument, computedStyle, docElem = doc.documentElement,
			body = doc.body, defaultView = doc.defaultView,
			prevComputedStyle = defaultView.getComputedStyle(elem, null),
			top = elem.offsetTop, left = elem.offsetLeft;

		while ( (elem = elem.parentNode) && elem !== body && elem !== docElem ) {
			computedStyle = defaultView.getComputedStyle(elem, null);
			top -= elem.scrollTop, left -= elem.scrollLeft;
			if ( elem === offsetParent ) {
				top += elem.offsetTop, left += elem.offsetLeft;
				if ( jQuery.offset.doesNotAddBorder && !(jQuery.offset.doesAddBorderForTableAndCells && /^t(able|d|h)$/i.test(elem.tagName)) )
					top  += ( computedStyle.borderTopWidth - 0 ) || 0,
					left += ( computedStyle.borderLeftWidth - 0 ) || 0;
				prevOffsetParent = offsetParent, offsetParent = elem.offsetParent;
			}
			if ( jQuery.offset.subtractsBorderForOverflowNotVisible && computedStyle.overflow !== "visible" )
				top  += ( computedStyle.borderTopWidth - 0 ) || 0,
				left += ( computedStyle.borderLeftWidth - 0 ) || 0;
			prevComputedStyle = computedStyle;
		}

		if ( prevComputedStyle.position === "relative" || prevComputedStyle.position === "static" )
			top  += body.offsetTop,
			left += body.offsetLeft;

		else if ( prevComputedStyle.position === "fixed" )
			top  += Math.max(docElem.scrollTop, body.scrollTop),
			left += Math.max(docElem.scrollLeft, body.scrollLeft);

	}
	return { top: top, left: left };
};
Attachments (1)
  • jquery-1.3.3.js (121.8 KB) - added by roviury March 08, 2009 01:35PM UTC.

    this is the result of improving jquery (according the jquery-1.3.2.js)

Change History (3)

Changed March 09, 2009 07:33AM UTC by dantman comment:1

Would it be possible for you to provide a diff that can actually be applied? I expect a large file like that is really hard for John and the others to apply because they don't know what was changed and you simply provided a modified jquery-1.3.2, when they are actually working with a trunk version of jQuery from svn which is actually broken up into about 6 separate files.

What you have is a compiled form of an old version of all 6 of those files combined together.

Changed April 21, 2009 07:20PM UTC by brandon comment:2

description: jQuery.fn.offset can be more simplify. \ parseInt is not necessary. \ it can change to... \ jQuery.fn.offset = function() { \ var elem = this[0]; \ if ( !elem ) return { top: 0, left: 0 }; \ var doc = elem.ownerDocument, body = doc.body; \ if ( elem === body ) return jQuery.offset.bodyOffset( elem ); \ if ( document.documentElement["getBoundingClientRect"] ){ \ var box = elem.getBoundingClientRect(), docElem = doc.documentElement, \ clientTop = docElem.clientTop || body.clientTop || 0, clientLeft = docElem.clientLeft || body.clientLeft || 0, \ top = box.top + (self.pageYOffset || jQuery.boxModel && docElem.scrollTop || body.scrollTop ) - clientTop, \ left = box.left + (self.pageXOffset || jQuery.boxModel && docElem.scrollLeft || body.scrollLeft) - clientLeft; \ }else{ \ jQuery.offset.initialized || jQuery.offset.initialize(); \ \ var offsetParent = elem.offsetParent, prevOffsetParent = elem, \ doc = elem.ownerDocument, computedStyle, docElem = doc.documentElement, \ body = doc.body, defaultView = doc.defaultView, \ prevComputedStyle = defaultView.getComputedStyle(elem, null), \ top = elem.offsetTop, left = elem.offsetLeft; \ \ while ( (elem = elem.parentNode) && elem !== body && elem !== docElem ) { \ computedStyle = defaultView.getComputedStyle(elem, null); \ top -= elem.scrollTop, left -= elem.scrollLeft; \ if ( elem === offsetParent ) { \ top += elem.offsetTop, left += elem.offsetLeft; \ if ( jQuery.offset.doesNotAddBorder && !(jQuery.offset.doesAddBorderForTableAndCells && /^t(able|d|h)$/i.test(elem.tagName)) ) \ top += ( computedStyle.borderTopWidth - 0 ) || 0, \ left += ( computedStyle.borderLeftWidth - 0 ) || 0; \ prevOffsetParent = offsetParent, offsetParent = elem.offsetParent; \ } \ if ( jQuery.offset.subtractsBorderForOverflowNotVisible && computedStyle.overflow !== "visible" ) \ top += ( computedStyle.borderTopWidth - 0 ) || 0, \ left += ( computedStyle.borderLeftWidth - 0 ) || 0; \ prevComputedStyle = computedStyle; \ } \ \ if ( prevComputedStyle.position === "relative" || prevComputedStyle.position === "static" ) \ top += body.offsetTop, \ left += body.offsetLeft; \ \ else if ( prevComputedStyle.position === "fixed" ) \ top += Math.max(docElem.scrollTop, body.scrollTop), \ left += Math.max(docElem.scrollLeft, body.scrollLeft); \ \ } \ return { top: top, left: left }; \ };jQuery.fn.offset can be more simplify. \ parseInt is not necessary. \ it can change to... \ {{{ \ jQuery.fn.offset = function() { \ var elem = this[0]; \ if ( !elem ) return { top: 0, left: 0 }; \ var doc = elem.ownerDocument, body = doc.body; \ if ( elem === body ) return jQuery.offset.bodyOffset( elem ); \ if ( document.documentElement["getBoundingClientRect"] ){ \ var box = elem.getBoundingClientRect(), docElem = doc.documentElement, \ clientTop = docElem.clientTop || body.clientTop || 0, clientLeft = docElem.clientLeft || body.clientLeft || 0, \ top = box.top + (self.pageYOffset || jQuery.boxModel && docElem.scrollTop || body.scrollTop ) - clientTop, \ left = box.left + (self.pageXOffset || jQuery.boxModel && docElem.scrollLeft || body.scrollLeft) - clientLeft; \ }else{ \ jQuery.offset.initialized || jQuery.offset.initialize(); \ \ var offsetParent = elem.offsetParent, prevOffsetParent = elem, \ doc = elem.ownerDocument, computedStyle, docElem = doc.documentElement, \ body = doc.body, defaultView = doc.defaultView, \ prevComputedStyle = defaultView.getComputedStyle(elem, null), \ top = elem.offsetTop, left = elem.offsetLeft; \ \ while ( (elem = elem.parentNode) && elem !== body && elem !== docElem ) { \ computedStyle = defaultView.getComputedStyle(elem, null); \ top -= elem.scrollTop, left -= elem.scrollLeft; \ if ( elem === offsetParent ) { \ top += elem.offsetTop, left += elem.offsetLeft; \ if ( jQuery.offset.doesNotAddBorder && !(jQuery.offset.doesAddBorderForTableAndCells && /^t(able|d|h)$/i.test(elem.tagName)) ) \ top += ( computedStyle.borderTopWidth - 0 ) || 0, \ left += ( computedStyle.borderLeftWidth - 0 ) || 0; \ prevOffsetParent = offsetParent, offsetParent = elem.offsetParent; \ } \ if ( jQuery.offset.subtractsBorderForOverflowNotVisible && computedStyle.overflow !== "visible" ) \ top += ( computedStyle.borderTopWidth - 0 ) || 0, \ left += ( computedStyle.borderLeftWidth - 0 ) || 0; \ prevComputedStyle = computedStyle; \ } \ \ if ( prevComputedStyle.position === "relative" || prevComputedStyle.position === "static" ) \ top += body.offsetTop, \ left += body.offsetLeft; \ \ else if ( prevComputedStyle.position === "fixed" ) \ top += Math.max(docElem.scrollTop, body.scrollTop), \ left += Math.max(docElem.scrollLeft, body.scrollLeft); \ \ } \ return { top: top, left: left }; \ }; \ }}}

Changed April 21, 2009 07:24PM UTC by brandon comment:3

resolution: → wontfix
status: newclosed

Does this give advantages besides just less code? I prefer to stick with parseFloat (has since been changed from parseInt to parseFloat) because it is more explicit about what it is doing. The implicit type conversion seems awkward/quirky and not immediately obvious while glancing through the code.