Ticket #11004 (closed bug: fixed)
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: | |
| Blocking: | Blocked by: |
Description (last modified by dmethvin) (diff)
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
Change History
comment:1 Changed 18 months ago by rwaldron
- Cc mikesherov added
- Component changed from unfiled to css
comment:2 Changed 18 months ago by jpate
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
comment:3 Changed 18 months ago by jpate
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)
comment:5 follow-up: ↓ 10 Changed 18 months ago by timmywil
- Status changed from new to closed
- Resolution set to invalid
We have 3 methods to manually account for padding, border, and margin. To retrieve a border-box width, use .outerWidth().
comment:6 Changed 18 months ago by anonymous
The problem is that animate() does not use these methods. Animations glitch out! This needs to be automatic
comment:7 Changed 18 months ago by jpate
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.
comment:8 Changed 18 months ago by mikesherov
- Status changed from closed to reopened
- Resolution invalid deleted
.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.
comment:9 Changed 18 months ago by mikesherov
- Owner set to mikesherov
- Status changed from reopened to assigned
comment:10 in reply to: ↑ 5 Changed 18 months ago by jpate
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
comment:11 Changed 18 months ago by mikesherov
- Priority changed from undecided to low
- Milestone changed from None to 1.next
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.
comment:12 follow-up: ↓ 13 Changed 18 months ago by 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.
comment:13 in reply to: ↑ 12 Changed 18 months ago by jpate
Replying to 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
comment:14 Changed 18 months ago by jpate
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
comment:15 Changed 18 months ago by jpate
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
comment:16 Changed 17 months ago by mikesherov
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.
comment:17 Changed 17 months ago by timmywil
- Cc mikesherov removed
- Keywords 1.8-discuss added
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().
comment:18 Changed 17 months ago by timmywil
+1, The change makes sense to me, but only if we can keep it between 0-30 bytes.
comment:19 Changed 17 months ago by mikesherov
- Description modified (diff)
+1, I'll handle this. Should actually be pretty small.
comment:20 Changed 17 months ago by dmethvin
- Description modified (diff)
+1, I don't *want* to deal with this but I think we *have* to.
comment:21 Changed 17 months ago by jaubourg
+1
comment:22 Changed 17 months ago by rwaldron
+1, As long as it's the right thing to do
comment:24 Changed 15 months ago by mikesherov
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.
comment:25 Changed 15 months ago by mikesherov
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.
comment:26 Changed 14 months ago by mikesherov
comment:27 Changed 12 months ago by dmethvin
- Status changed from assigned to closed
- Resolution set to fixed
Please follow the bug reporting guidlines and use jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

@mikesherov can you take a look at this?