Bug Tracker

Opened 12 years ago

Closed 12 years ago

#1796 closed bug (fixed)

.height() and .width() can be wrong if element has padding on all sides and is constrained by containing block

Reported by: dcrosta Owned by: brandon
Priority: major Milestone: 1.2.2
Component: core Version: 1.2.1
Keywords: Cc: dcrosta@…
Blocked by: Blocking:

Description

In this example:

<html>
<head>
<script type="text/javascript" src="http://code.jquery.com/jquery-latest.pack.js"></script>
<style>
.box {
    padding: 30px 15px;
    border:1px solid black;
}

.constraint {
    width:250px;
}
</style>
<script type="text/javascript">
    $(document).ready(function() {
        alert( $('.box').height() );
    });
</script>
</head>

<body>
    <div class="constraint">
    <div class="box">Lorem ipsum dolor sit amet, consectetuer adipiscing elit.
    Nulla ornare. Ut mattis porttitor felis. Fusce vitae magna vitae odio euismod
    lacinia. In non mi sed sem bibendum consectetuer.</div>
    </div>
</body>
</html>

Javascript alerts that the height is 100 pixels (in FF2/Win). However, firebug & visual inspection show the height is 120px. The cause is that the width of the div whose height we are checking is constrained by the containing block -- so when the .height() method removes the border & padding to check the size of the box, it gets re-layed-out by the browser.

I've attached a patch which removes only the side padding & border when checking height, and only the top/bottom padding & border when checking width. This resolves the issue for this test case.

Attachments (2)

jquery.patch (630 bytes) - added by dcrosta 12 years ago.
1796.diff (507 bytes) - added by davidserduke 12 years ago.
new patch

Download all attachments as: .zip

Change History (5)

Changed 12 years ago by dcrosta

Attachment: jquery.patch added

Changed 12 years ago by davidserduke

Attachment: 1796.diff added

new patch

comment:1 Changed 12 years ago by davidserduke

need: ReviewPatch

I went ahead and put together a slightly different patch since SVN has different code currently than the one dcrosta started with, but the idea is still identical.

comment:2 Changed 12 years ago by joern

Owner: set to brandon

Brandon should take a look at it. Maybe he can add a test to the new dimensions testsuite.

comment:3 Changed 12 years ago by brandon

Resolution: fixed
Status: newclosed

Fixed in Rev [4100].

Note: See TracTickets for help on using tickets.