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 comment:1
cc: | → mikesherov |
---|---|
component: | unfiled → css |
Changed December 13, 2011 02:36AM UTC by comment:2
Cleaned up the fork i've been working on
https://github.com/jamie-pate/jquery/commit/2ecd1a300c1caff78b6a4c41117f59e55af7537a
live test case at http://moscore.com/testbed/jquery/test_animate_box_sizing.html
Changed December 13, 2011 03:15AM UTC by 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:32PM UTC by comment:5
resolution: | → invalid |
---|---|
status: | new → closed |
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 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 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 comment:8
resolution: | invalid |
---|---|
status: | closed → reopened |
.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 comment:9
owner: | → mikesherov |
---|---|
status: | reopened → assigned |
Changed December 14, 2011 09:42PM UTC by 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/4216c62bf2db06897dd5be503a515e16256d9f2e → 1323900893234863 |
---|
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 comment:11
milestone: | None → 1.next |
---|---|
priority: | undecided → low |
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 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 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 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 \ \ oof → 1323972939360957 |
---|
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 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 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 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 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 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.js → 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 |
---|
+1, I'll handle this. Should actually be pretty small.
Changed December 19, 2011 05:08PM UTC by 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.js → 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 |
---|
+1, I don't *want* to deal with this but I think we *have* to.
Changed December 19, 2011 05:10PM UTC by comment:21
+1
Changed December 19, 2011 05:36PM UTC by comment:22
_comment0: | +1 → 1324317008181716 |
---|
+1, As long as it's the right thing to do
Changed January 18, 2012 12:01AM UTC by comment:23
milestone: | 1.next → 1.8 |
---|
Changed March 10, 2012 07:37PM UTC by 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 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 comment:26
Changed May 18, 2012 01:08AM UTC by comment:27
resolution: | → fixed |
---|---|
status: | assigned → closed |
@mikesherov can you take a look at this?