Bug Tracker

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#7948 closed bug (patchwelcome)

.offset() does not take body border into account

Reported by: jspath Owned by:
Priority: low Milestone: 1.next
Component: dimensions Version: 1.4.4
Keywords: Cc: scott_gonzalez
Blocked by: Blocking:

Description

.offset() does not seem to properly account for the body having a border set on it.

Here's an example: http://jsfiddle.net/TdZfh/3/

Change History (8)

comment:1 Changed 8 years ago by ajpiano

Component: unfileddimensions

according to scott gonzalez, this has been a known limitation for some time, but just not documented. in that case, it might just be a needsdoc. i am following up with brandon aaron, however, to see what the limitation is that is preventing us from taking border/margin/padding on the body into account and whether it is surmountable.

thanks for taking the time to file a ticket!

comment:2 Changed 8 years ago by brandon

I haven't looked back into this in some time. However, the offset method in dimensions used to take these factors into account and it was A LOT of code (see also forking and browser detection) to properly support it with all its quirks. Adding border/margin/padding to the body had very different results in the browsers depending on other conditions of the element. Here is an example from the original dimensions offset method:

// Mozilla does not include the border on body if an element isn't positioned absolute and is without an absolute parent
if (mo && parPos == 'absolute') absparent = true;
// IE does not include the border on the body if an element is position static and without an absolute or relative parent
if (ie && parPos == 'relative') relparent = true;

and another example

// Safari 2 and IE Standards Mode doesn't add the body margin for elments positioned with static or relative
if (((sf && !sf3) || (ie && $.boxModel)) && elemPos != 'absolute' && elemPos != 'fixed') {
    x += num(parent, 'marginLeft');
    y += num(parent, 'marginTop');
}
// Safari 3 does not include the border on body
// Mozilla does not include the border on body if an element isn't positioned absolute and is without an absolute parent
// IE does not include the border on the body if an element is positioned static and without an absolute or relative parent
if ( sf3 || (mo && !absparent && elemPos != 'fixed') || 
     (ie && elemPos == 'static' && !relparent) ) {
    x += num(parent, 'borderLeftWidth');
    y += num(parent, 'borderTopWidth');
}

Based on my previous experience in supporting this, the limited number of people actually styling the body element, and the simple workaround of using a nested div instead... I decided to not support this when moving dimensions to core.

comment:3 Changed 8 years ago by ajpiano

thanks for chiming in brandon. i personally am not convinced we shouldn't support this in core - but others probably have their own opinions. please do share your thoughts here...

comment:4 Changed 8 years ago by addyosmani

My personal take on this is that where some behavior is not used by a significant number of users (I'm both a designer and developer and have probably only seen this twice ever), it may not be the in the best interests of the project to invest time (and more importantly additional bytes/kbs to jQuery) to support it. Brandon's thoughts when initially looking at the problem (ie. a nested div workaround) probably sound like a reasonable alternative to suggest.

Just my two cents :)

Last edited 8 years ago by addyosmani (previous) (diff)

comment:5 Changed 8 years ago by jspath

Personally, I was able to work around this by using jQuery UI's position() method.

My alternatives were to manually add the border width myself, or set the border on a div instead of the body.

Obviously I'd be in favor of this being fixed ... but if it's not worth the effort some sort of note about this in the documentation is probably worth adding.

Before I asked about this IRC I went to the docs and even looked through the comments and didn't see any notice about this problem.

comment:6 Changed 8 years ago by addyosmani

Cc: scott_gonzalez added

CCing Scott Gonzalez for his thoughts on this as he may have more knowledge on position() than the rest of us. Scott, do you think this should be a fix or wontfix?

comment:7 Changed 8 years ago by addyosmani

Keywords: needsdocs added
Priority: undecidedlow
Resolution: patchwelcome
Status: newclosed

Jaubourg, DaveMethvin, scott_gonzalez and I discussed this and we think that given the perf-hits that would be involved in supporting this, we're currently happier providing detailed documentation on why it isnt supported instead of making all users suffer the delay in page load. If someone wants to make a case for it being included or has an idea for how it could be achieved in an optimal fashion, please feel free to re-open.

comment:8 Changed 8 years ago by danheberden

Keywords: needsdocs removed
Note: See TracTickets for help on using tickets.