Skip to content

Commit

Permalink
A bunch of fixes for offsets.
Browse files Browse the repository at this point in the history
  • Loading branch information
@savetheclocktower
savetheclocktower committed Nov 3, 2009
1 parent 7aa195b commit 97ea37d
Showing 1 changed file with 51 additions and 6 deletions.
57 changes: 51 additions & 6 deletions src/dom/layout.js
View file
Expand Up @@ -79,6 +79,18 @@
return true;
}

var hasLayout = Prototype.K;

if ('currentStyle' in document.documentElement) {
hasLayout = function(element) {
if (!element.currentStyle.hasLayout) {
element.style.zoom = 1;
}
return element;
};
}


/**
* class Element.Layout < Hash
*
Expand Down Expand Up @@ -411,7 +423,14 @@
* Element.Offset#inspect() -> String
**/
inspect: function() {
return "#<Element.Offset left: #{left} top: #{top}".interpolate(this);
return "#<Element.Offset left: #{left} top: #{top}>".interpolate(this);
},

/**
* Element.Offset#toString() -> String
**/
toString: function() {
return "[#{left}, #{top}]".interpolate(this);
},

/**
Expand Down Expand Up @@ -477,17 +496,24 @@
* (the element that would be returned by [[Element.getOffsetParent]]).
**/
function positionedOffset(element) {
// Account for the margin of the element.
var layout = element.getLayout();

var valueT = 0, valueL = 0;
do {
valueT += element.offsetTop || 0;
valueL += element.offsetLeft || 0;
element = element.offsetParent;
if (element) {
if (element.tagName.toUpperCase() == 'BODY') break;
if (isBody(element)) break;
var p = Element.getStyle(element, 'position');
if (p !== 'static') break;
}
} while (element);

valueL -= layout.get('margin-top');
valueT -= layout.get('margin-left');

return new Element.Offset(valueL, valueT);
}

Expand Down Expand Up @@ -544,6 +570,10 @@
viewportOffset: viewportOffset
});

function isBody(element) {
return $w('BODY HTML').include(element.nodeName.toUpperCase());

This comment has been minimized.

Sign in to view

Copy link
@kangax

kangax Nov 3, 2009

Collaborator

This is pretty slow and eats memory. The sugar is not worth perf. loss here, imo. Can we replace this with — var nodeName = element.nodeName.toUpperCase(); return nodeName === 'BODY' || nodeName === 'HTML'? I would also rename this method to — isRootElement, following CSS terminology.

This comment has been minimized.

Sign in to view

Copy link
@savetheclocktower

savetheclocktower Nov 5, 2009

Author Collaborator

I'm going to rewrite this function entirely. I might not even need it.
}

// If the browser supports the nonstandard `getBoundingClientRect`
// (currently only IE and Firefox), it becomes far easier to obtain
// true offsets.
Expand All @@ -565,11 +595,26 @@
positionedOffset: function(element) {
element = $(element);
var parent = element.getOffsetParent();
var isBody = (parent.nodeName.toUpperCase() === 'BODY');

// When the BODY is the offsetParent, IE6 mistakenly reports the

This comment has been minimized.

Sign in to view

Copy link
@staaky

staaky Nov 4, 2009

That bug is convenient here, but getOffsetParent should probably do the right thing and return the BODY element on IE6. I remember that it was once fixed but it was reverted to clean up a 1.6.0.3 release.

Could still use the logic behind the bug to test but perhaps there's a better way. What quirk does this fix?

This comment has been minimized.

Sign in to view

Copy link
@savetheclocktower

savetheclocktower Nov 5, 2009

Author Collaborator

You're right. I'll fix the behavior of getOffsetParent instead.

This fixes a quirk where getBoundingClientRect doesn't return proper offsets for the BODY tag. The margin of error is weird but predictable (something like 8px off horizontally and 12px off vertically).
// parent as HTML. Use that as the litmus test to fix another
// annoying IE6 quirk.
if (parent.nodeName.toUpperCase() === 'HTML') {
return positionedOffset(element);
}

var eOffset = element.viewportOffset(),
pOffset = isBody ? viewportOffset(parent) : parent.viewportOffset();
return eOffset.relativeTo(pOffset);
pOffset = isBody(parent) ? viewportOffset(parent) :
parent.viewportOffset();
var retOffset = eOffset.relativeTo(pOffset);

// Account for the margin of the element.
var layout = element.getLayout();
var top = retOffset.top - layout.get('margin-top');
var left = retOffset.left - layout.get('margin-left');

return new Element.Offset(left, top);
}
});
});
}
})();

2 comments on commit 97ea37d

@dperini
Copy link

@dperini dperini commented on 97ea37d Nov 3, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dig the suggested "isRootElement" name !

This is only a problem in IE browsers so I still think that for those browsers a load time feature test is better.
FT should check which one of HTML or BODY is considered the actual "root" element in that browser and in that specific page.
In IE I have successfully used the "compatMode" property or "DOCTYPE" sniffing using "systemId" and "publicId" properties of "doctype" object.

@kangax
Copy link
Collaborator

@kangax kangax commented on 97ea37d Nov 3, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dperini What about:

function isRootElement(element) { 
  return element.tagName.toUpperCase() === 'HTML'; 
}
// is BODY a root element?
if (document.documentElement.clientWidth === 0) {
  isRootElement = function(element){ 
    return element.tagName.toUpperCase() === 'BODY'; 
  };
}

However, I think this won't really work with multiple documents (where an element whose tagName is "HTML" might not be a root ancestor of an element in question).

Please sign in to comment.