Skip to main content

Bug Tracker

Side navigation

#11004 closed bug (fixed)

Opened December 12, 2011 09:33PM UTC

Closed May 18, 2012 01:08AM UTC

getWH incorrectly removes padding and border width when box-sizing is border-box

Reported by: jpate Owned by: mikesherov
Priority: low Milestone: 1.8
Component: css Version: 1.7.1
Keywords: 1.8-discuss Cc:
Blocked by: Blocking:
Description

tested in jquery 1.6.4, 1.7.1

tested in chrome 15.0.874.121 m win7 x64

to reproduce: create a div with the following style:

box-sizing:border-box;width:50;padding:10px;margin:10px solid red

call jQuery(elem)animate({width:50;height:50})

the box will jump to the wrong starting size, then animate

test case: http://jsfiddle.net/GUfuf/6/

patch: https://github.com/jamie-pate/jquery/commit/72bd44aebb10a66f2d7d51f87bc398f5d6541b95#src/css.js

Attachments (0)
Change History (27)

Changed December 12, 2011 10:09PM UTC by rwaldron comment:1

cc: → mikesherov
component: unfiledcss

@mikesherov can you take a look at this?

Changed December 13, 2011 02:36AM UTC by jpate comment:2

Changed December 13, 2011 03:15AM UTC by jpate comment:3

Tests completed in 120276 milliseconds.

5839 tests of 5839 passed, 0 failed.

Not sure what to do about ie6/7 though, since they don't support box-sizing in a sane way (and how to detect ie 8/9 running in ie7 standards mode)

Changed December 14, 2011 02:31PM UTC by timmywil comment:4

#11009 is a duplicate of this ticket.

Changed December 14, 2011 02:32PM UTC by timmywil comment:5

resolution: → invalid
status: newclosed

We have 3 methods to manually account for padding, border, and margin. To retrieve a border-box width, use .outerWidth().

Changed December 14, 2011 07:47PM UTC by anonymous comment:6

The problem is that animate() does not use these methods. Animations glitch out! This needs to be automatic

Changed December 14, 2011 07:56PM UTC by jpate comment:7

The problem is that animate() does not use these methods. Animate uses the cssHooks for 'width' and 'height' to determine the starting size of an element.

The actual size of an element is different when box-sizing:border-box is used. the CORRECT size does not include padding or border! Therefore the starting size of the animation is wrong and it looks glitchy. Please take a look at the test case(s) I have provided.

Changed December 14, 2011 08:18PM UTC by mikesherov comment:8

resolution: invalid
status: closedreopened

.width() and getWH always returns content width regardless of box-sizing.

Note the warning on http://api.jquery.com/width/

"Note that .width() will always return the content width, regardless of the value of the CSS box-sizing property" This is intended behavior for .width(). However, if animation needs to do something differently, is should probably be using .css('width').

I'll take a look.

Changed December 14, 2011 08:18PM UTC by mikesherov comment:9

owner: → mikesherov
status: reopenedassigned

Changed December 14, 2011 09:42PM UTC by jpate comment:10

_comment0: Here is my new proposed patch, it fixes the problem by swapping out the box model when calculating the starting point of animations. Tested in ie8/9, opera 11.60, firefox 8, firefox 3.6, chrome 15.0.874.121 m, safari 5.1.2 (all on w7 x64) \ \ https://github.com/jamie-pate/jquery/commit/4216c62bf2db06897dd5be503a515e16256d9f2e1323900893234863

Here is my new proposed patch, it fixes the problem by swapping out the box model when calculating the starting point of animations. Tested in ie8/9, opera 11.60, firefox 8, firefox 3.6, chrome 15.0.874.121 m, safari 5.1.2 (all on w7 x64)

https://github.com/jamie-pate/jquery/tree/animate-swap-box-sizing-content-box

i've also added the fix to my test case at http://moscore.com/testbed/jquery/test_animate_box_sizing.html

Changed December 15, 2011 03:37AM UTC by mikesherov comment:11

milestone: None1.next
priority: undecidedlow

there will be a patch for css vendor prefix names shortly. At that point, I think we can figure out how to move forward on this. I'm not a fan of using swap because it's slow, but I think there is a solution here.

Changed December 15, 2011 06:14AM UTC by mikesherov comment:12

css3 prefix patch: https://github.com/jquery/jquery/pull/639

I am still going to be looking for a more comprehensive solution to this bug.

Changed December 15, 2011 05:58PM UTC by jpate comment:13

_comment0: Replying to [comment:12 mikesherov]: \ > css3 prefix patch: https://github.com/jquery/jquery/pull/639 \ > \ > I am still going to be looking for a more comprehensive solution to this bug. \ \ by that do you mean that the bug covers more than just animation?1323971971211217

Replying to [comment:12 mikesherov]:

css3 prefix patch: https://github.com/jquery/jquery/pull/639 I am still going to be looking for a more comprehensive solution to this bug.

is it that slow on 2 properties? :) I guess testing is in order

Changed December 15, 2011 06:15PM UTC by jpate comment:14

_comment0: 1000 iterations: $(div).stop(true).animate(size++,size++) \ 1.7.2pre_swap_patch 2325ms \ 1.7.1 1222ms \ 1.7.2pre_getwh_patch 1606ms \ \ oof1323972939360957

1000 iterations: $(div).stop(true).animate(size++,size++)

1.7.2pre_swap_patch 2325ms

1.7.1 1222ms

1.7.2pre_getwh_patch 1606ms

oof

Changed December 15, 2011 07:57PM UTC by jpate comment:15

_comment0: Unfortunately swap works differently in chrome/safari vs android mobile, swapping "WebkitBoxSizing" breaks chrome/safari box sizing, but is required to fix this issue in android mobile! \ \ '''frustrating'''1323981269483446

Unfortunately swap works differently in chrome/safari vs android mobile, swapping "WebkitBoxSizing" breaks chrome/safari box sizing, but is required to fix this issue in android mobile!

frustrating

turns out this is a 'bug' in swap(), WebkitBoxSizing and BoxSizing are actually the same object in webkit, changing one causes the other to change and the new value is written to the old object

Changed December 16, 2011 01:54PM UTC by mikesherov comment:16

jpate, what's I'm currently thinking is that .css('width') should take into account box-sizing while leaving .width() to still return the .width() property.

All other solutions I've seen or come up with are slow hacks. Please bear with me as there are a few other tickets that need to get integrated first to make this happen.

Changed December 16, 2011 03:14PM UTC by timmywil comment:17

cc: mikesherov
keywords: → 1.8-discuss

Please vote here on whether .css('width') and .width() should return different things based on border-box. We can safely say that .width() should never take box-sizing into account when calculating width. width/innerWidth/outerWidth/outerWidth(true) provide all of the necessary options for adding/subtracting padding/border/margin. However, if .css('width') takes box-sizing into account, it will solve some issues (for instance, when animating), but will no longer be the same as .width().

Changed December 16, 2011 03:16PM UTC by timmywil comment:18

+1, The change makes sense to me, but only if we can keep it between 0-30 bytes.

Changed December 16, 2011 08:10PM UTC by mikesherov comment:19

description: tested in jquery 1.6.4, 1.7.1 \ \ tested in chrome 15.0.874.121 m win7 x64 \ \ to reproduce: create a div with the following style: \ box-sizing:border-box;width:50;padding:10px;margin:10px solid red \ \ call jQuery(elem)animate({width:50;height:50}) \ \ the box will jump to the wrong starting size, then animate \ \ test case: http://jsfiddle.net/GUfuf/6/ \ \ patch: https://github.com/jamie-pate/jquery/commit/72bd44aebb10a66f2d7d51f87bc398f5d6541b95#src/css.jstested in jquery 1.6.4, 1.7.1\ \ tested in chrome 15.0.874.121 m win7 x64\ \ to reproduce: create a div with the following style:\ box-sizing:border-box;width:50;padding:10px;margin:10px solid red\ \ call jQuery(elem)animate({width:50;height:50})\ \ the box will jump to the wrong starting size, then animate\ \ test case: http://jsfiddle.net/GUfuf/6/\ \ patch: https://github.com/jamie-pate/jquery/commit/72bd44aebb10a66f2d7d51f87bc398f5d6541b95#src/css.js

+1, I'll handle this. Should actually be pretty small.

Changed December 19, 2011 05:08PM UTC by dmethvin comment:20

description: tested in jquery 1.6.4, 1.7.1\ \ tested in chrome 15.0.874.121 m win7 x64\ \ to reproduce: create a div with the following style:\ box-sizing:border-box;width:50;padding:10px;margin:10px solid red\ \ call jQuery(elem)animate({width:50;height:50})\ \ the box will jump to the wrong starting size, then animate\ \ test case: http://jsfiddle.net/GUfuf/6/\ \ patch: https://github.com/jamie-pate/jquery/commit/72bd44aebb10a66f2d7d51f87bc398f5d6541b95#src/css.jstested in jquery 1.6.4, 1.7.1 \ \ tested in chrome 15.0.874.121 m win7 x64 \ \ to reproduce: create a div with the following style: \ box-sizing:border-box;width:50;padding:10px;margin:10px solid red \ \ call jQuery(elem)animate({width:50;height:50}) \ \ the box will jump to the wrong starting size, then animate \ \ test case: http://jsfiddle.net/GUfuf/6/ \ \ patch: https://github.com/jamie-pate/jquery/commit/72bd44aebb10a66f2d7d51f87bc398f5d6541b95#src/css.js

+1, I don't *want* to deal with this but I think we *have* to.

Changed December 19, 2011 05:10PM UTC by jaubourg comment:21

+1

Changed December 19, 2011 05:36PM UTC by rwaldron comment:22

_comment0: +11324317008181716

+1, As long as it's the right thing to do

Changed January 18, 2012 12:01AM UTC by mikesherov comment:23

milestone: 1.next1.8

Changed March 10, 2012 07:37PM UTC by mikesherov comment:24

So, this bug in Firefox puts a wrinkle in things: https://bugzilla.mozilla.org/show_bug.cgi?id=520992

I can either now make a support test for this behavior, or I can modify the behavior of the fix I'm doing for 10413 to also respect box-sizing for .css('width'). I'm choosing the latter for now, as it'll be smaller overall.

Changed March 11, 2012 06:30PM UTC by mikesherov comment:25

I have the solution ready to go that also deals with https://bugzilla.mozilla.org/show_bug.cgi?id=520992 I'm just now trying to find a way to make it smaller. It ended up having to be built on top of #10413.

Changed April 10, 2012 02:37AM UTC by mikesherov comment:26

Changed May 18, 2012 01:08AM UTC by dmethvin comment:27

resolution: → fixed
status: assignedclosed