Skip to main content

Bug Tracker

Side navigation

#14084 closed bug (fixed)

Opened June 30, 2013 03:25PM UTC

Closed September 06, 2013 03:29PM UTC

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

Reported by: tj@crowdersoftware.com Owned by: m_gol
Priority: low Milestone: 1.11/2.1
Component: support Version: 1.10.1
Keywords: Cc:
Blocked by: Blocking:
Description

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.

Attachments (0)
Change History (33)

Changed July 01, 2013 04:37PM UTC by m_gol comment:1

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/

Changed July 01, 2013 04:47PM UTC by dmethvin comment:2

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

Changed July 01, 2013 05:12PM UTC by tj@crowdersoftware.com comment:3

@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.

Changed July 01, 2013 05:15PM UTC by tj@crowdersoftware.com comment:4

@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.

Changed July 01, 2013 05:18PM UTC by tj@crowdersoftware.com comment:5

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?

Changed July 01, 2013 05:31PM UTC by m_gol comment:6

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.

Changed July 01, 2013 05:41PM UTC by m_gol comment:7

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.

Changed July 01, 2013 06:27PM UTC by m_gol comment:8

summary: `height`/`width` documentation incorrect about `css` and box-sizing: border-boxelem.css('width') provides incorrect output with `box-sizing: border-box` if run before document ready

Changed July 01, 2013 06:40PM UTC by timmywil comment:9

component: unfiledsupport
priority: undecidedlow
status: newopen

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.

Changed July 01, 2013 06:42PM UTC by m_gol comment:10

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...

Changed July 01, 2013 06:44PM UTC by m_gol comment:11

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

Changed July 01, 2013 07:11PM UTC by timmywil comment:12

_comment0: 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. [http://stackoverflow.com/questions/1438883/jquery-why-use-document-ready-if-external-js-at-bottom-of-page 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 [http://www.w3.org/TR/html5/syntax.html#the-end difference], according to the spec. Any scripts using the `defer` flag are executed _before_ the ready event, but not before scripts at the end of the body. Just good to be aware of that. \ \ Finally, in oldIE, there was the [http://blogs.msdn.com/b/ie/archive/2008/04/23/what-happened-to-operation-aborted.aspx operation aborted event], but that could be avoided by ensuring the script tag's parent is actually the body. \ 1372706747781617

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.

Changed July 01, 2013 09:36PM UTC by tj@crowdersoftware.com comment:13

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.

Changed July 01, 2013 09:39PM UTC by dmethvin comment:14

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.

Changed July 01, 2013 10:07PM UTC by tj@crowdersoftware.com comment:15

@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?

Changed July 01, 2013 10:11PM UTC by dmethvin comment:16

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.

Changed July 01, 2013 10:40PM UTC by timmywil comment:17

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.

Changed July 09, 2013 09:03AM UTC by tj@crowdersoftware.com comment:18

@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].)

Changed July 09, 2013 09:14AM UTC by tj@crowdersoftware.com comment:19

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.

Changed July 09, 2013 02:36PM UTC by gibson042 comment:20

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.

Changed July 09, 2013 02:58PM UTC by T.J. Crowder comment:21

@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.

Changed July 09, 2013 07:25PM UTC by dmethvin comment:22

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.

Changed July 09, 2013 09:00PM UTC by gibson042 comment:23

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.

Changed July 09, 2013 09:16PM UTC by T.J. Crowder comment:24

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.

Changed August 19, 2013 05:56PM UTC by m_gol comment:25

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

Changed August 20, 2013 09:22AM UTC by T.J. Crowder comment:26

@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.

Changed August 20, 2013 06:15PM UTC by dmethvin comment:27

#14253 is a duplicate of this ticket.

Changed August 21, 2013 10:20PM UTC by m_gol comment:28

owner: → m_gol
status: openassigned

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).

Changed August 21, 2013 10:20PM UTC by m_gol comment:29

milestone: None1.11/2.1

Changed September 01, 2013 10:12PM UTC by mikesherov comment:30

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.

Changed September 01, 2013 11:26PM UTC by mikesherov comment:31

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.

Changed September 02, 2013 12:02AM UTC by m_gol comment:32

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

Changed September 06, 2013 03:29PM UTC by Michał Gołębiowski comment:33

resolution: → fixed
status: assignedclosed

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

Changeset: 776012b8b3898fad2e0727880f1dc4af5c7b33c1