elem.css('width') provides incorrect output with `box-sizing: border-box` if run before document ready

[tj@…]

I believe the current documentation for height and width is incorrect vis-a-vis what .css("height") and .css("width") do with box-sizing: border-box elements. The current documentation for height says:

Note that .height() will always return the content height, regardless of the value of the CSS box-sizing property. As of jQuery 1.8, this may require retrieving the CSS height plus box-sizing property and then subtracting any potential border and padding on each element when the element has box-sizing: border-box. To avoid this penalty, use .css("height") rather than .height().

(My emphasis. And similarly for .width / .css('width').)

This suggests that .css("height") will return something different from .height(), but it returns exactly the same thing (plus units), regardless of whether the element is using border-box or content-box (I didn't look at padding-box). (And similarly for .css("width") and .width().)

For example, this:

<style>
  div {
      height: 120px;
      width: 100px;
      padding-top: 20px;
      padding-left: 10px;
      background-color: #abc;
      border: 1px solid black;
  }
  .box {
      box-sizing: border-box;
      -moz-box-sizing: border-box;
  }
  .content {
  }
</style>
<div class="box"></div>
<div class="content"></div>
<script>
    showDimensions('box');
    showDimensions('content');
      
    function showDimensions(cls) {
        var elm = $('.' + cls),
            label = "$('." + cls + "')";

        console.log(label + ".height() = " + elm.height());
        console.log(label + ".css('height') = " + elm.css('height'));
  
        console.log(label + ".width() = " + elm.width());
        console.log(label + ".css('width') = " + elm.css('width'));
    }
</script>

...gives us:

$('.box').height() = 98
$('.box').css('height') = 98px
$('.box').width() = 88
$('.box').css('width') = 88px
$('.content').height() = 120
$('.content').css('height') = 120px
$('.content').width() = 100
$('.content').css('width') = 100px

​Live Example | ​Live Source (using v1.10.1)

Also, the behavior is the same in v1.6.4 (​demo), v1.7.2 (​demo), v1.8.3 (​demo), v1.9.1 (​demo), and v1.10.1 (above), whereas the documentation suggests some change occurred in v1.8.

[m_gol]

I can't confirm it on jsFiddle, weird:
1.x edge: ​http://jsfiddle.net/m_gol/kDVyv/
1.8.3: ​http://jsfiddle.net/m_gol/kDVyv/2/

[dmethvin]

That is really strange, I played with the jsbin version and it does look broken, but the jsFiddle is what I expected.

[tj@…]

@m_gol, @dmethvin: That is really strange! I can confirm that it behaves differently on jsFiddle as well (I get different numbers rather than the same ones), which is...well...strange.

I don't seem to have the rights to add a file attachment, but ​this completely standalone version replicates the issue for me (numbers are the same, like jsbin), both with edge and 1.8.3.

Hopefully it's just me doing something silly, but it's not immediately obvious to me what.

[tj@…]

@m_gol, @dmethvin: The difference is onload. jsFiddle defaults to putting your code in a window.load event handler (a very, very strange default to my way of thinking). Make it "No wrap - body" and the problem gets replicated: ​v1.8.3, ​edge.

[tj@…]

Sorry, click-itis: Problem does not replicate when using ready: ​v1.8.3, ​edge.

So rather than a documentation problem, is this an issue with css('height') and css('width') when used at bottom of page?

[m_gol]

This is because jQuery runs some tests after document ready fired and one of these tests is checking boxSizing support. jsFiddle must be injecting scripts asynchronously while jsBin & static pages don't; inline code is executed before tests run which makes jQuery believe boxSizing is not supported in this browser. If you put this code in a document ready call, it works.

We should fix it, IMHO. It seems intuitive for users to be able to assume that JS code put at the end of body doesn't need to wait for document ready. This assumption isn't always true as we can see in this example.

[m_gol]

We could "fix" it for 2.x by setting jQuery.support.boxSizing to true as all supported by 2.x browsers have it true.

It would be best to actually remove this property but I believe we need it for backwards compatibility with 1.x where it's needed - unfortunately jQuery.support is exposed. :(

As for 1.x, I don't see any quick & pretty solution.

[Timmy Willison]

iirc, assuming the document is ready at the end of the body can have other issues as well, irrespective of whether jQuery is on the page. But perhaps that was only oldIE? One things seems certain from our experience: we should not let this ticket encourage us to pull support tests requiring the body out of document ready. We've done this before and it was fraught with problems.

[m_gol]

That's interesting, Timmy, can you share info about some problems of this nature? I've seen suggestions that putting scripts at the end of body is equivalent to invoking them on doc ready so many times...

[m_gol]

And what do you think about assuming jQuery.support.boxSizing === true in 2.x?

[Timmy Willison]

Did a little searching. I was vaguely recalling a few things.

It is true that scripts running at the end of the body is basically the same as putting them at the end of the body, I would not officially recommend it because there are several conditions that could cause unwanted behavior. ​This answer on SO goes in more detail. Putting scripts at the end of the body is the same because ALL of the HTML source has been parsed by the browser. The point of the post is, be really sure you're not using something that hasn't been parsed yet (scripts using document.write or script tags anywhere besides the end of the body can cause errors).

Having said that, there is a ​difference, according to the spec. Any scripts using the defer flag are executed _before_ the ready event (under certain circumstances), but not before scripts at the end of the body. Just good to be aware of that.

Finally, in oldIE, there was the ​operation aborted event, but that could be avoided by ensuring the script tag's parent is actually the body.

But yes, I'd be fine with defaulting to true.

[tj@…]

Given the oft-repeated ​recommendation to put scripts at the end of body, barring very, very solid reasons contrary, I wouldn't recommend jQuery get things wrong in that situation. Certainly there's no reason this box sizing detect can't be more proactive.

[dmethvin]

Certainly there's no reason this box sizing detect can't be more proactive.

Great, can you submit a more proactive patch? The trick is supporting 1.x I think.

[tj@…]

@dmethvin:

Great, can you submit a more proactive patch?

Sure! (And thanks for the invite.) Give me a few days to shake some time free, particularly busy work schedule this week.

What's best, fork on github and do a pull request?

[dmethvin]

Definitely fork and pull request, more information is at ​http://contribute.jquery.org if you need it. Many thanks for your past bug reports, they're exemplars. We'd love to have your name in AUTHORS.txt.

[Timmy Willison]

Just to be clear, no one is arguing against putting scripts at the end of the body. The issue is document ready and whether we can get around needing a body for certain support tests.

[tj@…]

@dmethvin, @m_gol: I'd like to get your input on this if I could.

As you know, there are two relevant support properties: boxSizing and boxSizingReliable. Currently, the pre-ready code doesn't set boxSizing at all but sets boxSizingReliable to true, then the ready code sets boxSizing and optionally changes boxSizingReliable to false.

Re boxSizing:

The current test uses measurement, and so waits for body (the measurement doesn't work when the element is disconnected). I've tried several browsers* and it seems like the fairly common feature-sniff approach of testing for the property on an element's style works reliably before body exists. E.g., prior to the ready handler:

$.each('boxSizing MozBoxSizing msBoxSizing webkitBoxSizing OBoxSizing'.split(' '), function(i, prop) {
    if (prop in div.style) {
        support.boxSizing = true;
        return false;
    }
});

or more briefly but less readably

$.each('boxSizing MozBoxSizing msBoxSizing webkitBoxSizing OBoxSizing'.split(' '), function(i, prop) {
    return !(support.boxSizing = prop in div.style);
});

where div is an element that already exists in the function for various other tests.

Does anyone know of a reason I shouldn't do that? Known invalid results or similar? It would also have the advantage that it wouldn't be subject to bug #13543, so no need for the swapping test that fixed it (nice one, @m_gol).

Re boxSizingReliable:

boxSizingReliable gets set to false on ready based on a call to getComputedStyle to check whether it's giving the wrong values. The only browser I have handy where boxSizingReliable is false is Firefox (I have v3.6.15 and v20).

This is more problematic, because the div we check needs to be in the document for us to use getComputedStyle on it. I did have an idea, but I'm not that comfortable with it. [ That could be because I'm a bit outside my comfort zone when we get to browser edge cases. :-) ] The div needs to be in the document, but apparently it doesn't have to be in body. On the browsers I've checked, before ready I can append it to documentElement and do the test successfully, getting the expected values on Firefox (false) and Chrome/Safari/Opera (true) (the test is irrelevant to IE at present). This doesn't (in my tests) trigger creation of the body element like it would if the parser hit a <div> tag in the source. In fact, if I don't remove it, it remains a direct child of the html element even after page load is complete (even on IE! but of course we wouldn't need to do this on IE until/unless it supports getComputedStyle).

I don't see us appending elements to the documentElement (temporarily) anywhere else in the source, but provided it doesn't have unpleasant side-effects in browsers I haven't tested, it might be a viable option. But again, when it comes to the vagaries of potential cross-browser issues when appending to the document element (and temporarily creating an invalid structure) prior to body being created, I get out of my depth.

Thoughts?

Thanks - T.J. :-)

(* Browsers tested so far: IE6, IE8, IE10, Chrome 26 [Linux], Firefox 3.6.15 [Windows], Firefox 20 [Linux], Safari [iOS 6], Opera 12.15 [Linux].)

[tj@…]

Follow-up thought: The use-case we're trying to support is people like me who put scripts at the end of body, right? So pretty much my entire post just now was a bit silly.

The simpler solution is to test if body exists (prior to the ready callback) and if so, do the current boxSizing / boxSizingReliable tests right then; if not, do the current tests on ready. That supports:

1. Users putting script tags in head and using ready (or onload)

2. Users putting script tags in body and running them immediately (or using ready or onload)

It would not support users putting the script tag for jQuery in the head and then putting their own scripts in the body and not waiting for ready. I would think "don't do that" would be an acceptable answer to that scenario. :-) (If we really wanted to support that, we could trigger the tests on the first user-generated call into jQuery, but to me at that point we're going too far.)

To me, this is much less risky than appending the div to documentElement. It's actually a very small change.

[gibson042]

Actually, we do already append to documentElement during initialization: ​https://github.com/jquery/sizzle/blob/70f333eb1f62a779bffa748543b36093e8832bd0/sizzle.js#L509

I personally am all for taking advantage of the technique in jQuery support testing as well.

[T.J. Crowder]

@dgibson: Doh! That's what I get for filtering out Sizzle results in my search. :-)

That's fantastic, it means it's tried and true. (And very useful to know...)

So we have two options:

1. Use the "in style" test for boxSizing (the current check doesn't work earlier in IE, not even when the div is appended to documentElement) and move the boxSizingReliable test (my long comment earlier today), or

2. Detect whether body is already present and do the existing tests, deferring them until the ready handler if body isn't present (my shorter follow-up).

I don't think either is bigger in terms of code than the other to a degree that matters.

I prefer Option 1 unless someone has a known invalid result on the in test, because it handles use-cases Option 2 doesn't handle and doesn't vary at runtime depending on where the script tag is put. But I'm very new here and defer to those who aren't.

Once we have a consensus, I'll put together the patch (with tests, of course) and send a pull req.

-- T.J.

[dmethvin]

Option 1 sounds good. I recall that oldIE leaks memory when the in operator is used with host objects like window or document, dunno if that could be an issue here.

[gibson042]

I also prefer option 1, and would further note (though not necessarily recommend as in-scope for this fix) that documentElement.append( container ) can probably cover all presently-deferred support tests.

[T.J. Crowder]

Good deal, I'm on it. Just for boxSizing for now, but I'll look at the other deferred tests as a follow-on effort to see if we can move them up.

[m_gol]

T.J. Crowder, how is your work going? I can pick up this ticket if you didn't find time.

[T.J. Crowder]

@m_gol: I really wanted to be the one to fix this, but my plate is clearly overflowing (and running onto the floor, frankly). If you have the bandwidth in the next week, go for it. :-) If not, I'll see if I can pick it up on Sunday.

[dmethvin]

#14253 is a duplicate of this ticket.

[m_gol]

I'm taking it. I'll do it as part of my refactoring the support module (i.e. breaking it to tests performed in separate modules).

[mikesherov]

boxSizingReliable is irrelevant as of FF23. According to our support statement, it can be removed when FF24 comes out. Also, the case in which its hit is quite rare (and involves a sized but disconnected element). I have a PR to remove it once FF24 lands.

[mikesherov]

BTW, setting boxSizing by default to true is something we should do as m_gol stated, even for 1.x. True is the correct assumption for like 95% of pageloads on the net.

[m_gol]

@mikesherov boxSizingReliable is needed for IE10 and IE11 as well, I'm afraid. :(

[Michał Gołębiowski]

Fix #14084: attach the test div to documentElement, not body.

Changeset: 776012b8b3898fad2e0727880f1dc4af5c7b33c1