Side navigation
#11415 closed enhancement (fixed)
Opened February 28, 2012 09:27PM UTC
Closed March 02, 2012 03:32AM UTC
Last modified March 17, 2012 01:25AM UTC
Silently ignore negative CSS values where they are illegal
| Reported by: | SineSwiper | Owned by: | SineSwiper |
|---|---|---|---|
| Priority: | low | Milestone: | 1.7.2 |
| Component: | effects | Version: | 1.7.1 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
Currently, there is a browser normalization problem with the way negative values are treated between IE and Firefox. Firefox will silently ignore negative CSS values where they are not appropriate. IE will error out.
While "allowing" an illegal value for a property may seem something less to do with jQuery and more to do with the JS programmer, these kind of problems can occur in a single frame of animation, and go unnoticed until a check in IE reveals the problem. One such example is the jQuery UI bug #8142, which uses an easing of 'easeInOutElastic' to inadvertently cause a negative height problem.
There have been some fixed bugs in the past to take care of these piecemeal and has been a source of dupe bugs, such as #3881, #4413, #5051, #5588, #10294, #10582. However, the css(), prop(), and animate() should go ahead and take care of all of the potential cases for this problem and eliminate all of them at the same time.
Based on my research, the following CSS properties cannot contain negative values, according to the W3C CSS2.1 standard (and certain CSS3 modules):
- border-spacing
- padding-top, padding-right, padding-bottom, padding-left, and padding
- border-top-width, border-right-width, border-bottom-width, border-left-width, and border-width
- border-top, border-right, border-bottom, border-left, and border (if specifying a width)
- font-size and font (if specifying a size)
- width and height
- min-width, max-width, min-height, and max-height
- line-height
- outline-width and outline (if specifying a width)
- widows
CSS3:
- column-gap and column-rule-width
- device-width, device-height, color, color-index, monochrome
- marquee-play-count
- background-size, border-image-slice, border-image-width, border-image-outset, box-shadow (only the blur radius)
Attachments (0)
Change History (6)
Changed February 28, 2012 09:38PM UTC by comment:1
| _comment0: | As a thought exercise, if it were Chrome or Firefox that was picky would you want this behavior? Is IE wrong about the error or is the code wrong in a possibly-benign way and other browsers ignored it? \ \ With `.css("border: 1px 1px -1px 4px")`, are we supposed to parse the shorthand property and look for negative numbers? A more horrifying fix IMO would be to wrap it all in a try/catch and ignore any hint of an error coming out of the property assignment. \ \ I think our goals for cross-browser consistency should cover valid inputs, not invalid ones. Invalid inputs are errors and should be fixed. \ → 1330465149890173 |
|---|---|
| owner: | → SineSwiper |
| status: | new → pending |
Changed February 29, 2012 02:58PM UTC by comment:2
| status: | pending → new |
|---|
The fact that it's a browser inconsistency does add weight to the problem. However, even if Chrome/Firefox did the same thing, we'd still want to make sure that the automations of animations and the like are as painless as possible. Part of jQuery's magic is being able to do stuff to even zero elements and not giving you an "Object required" error. The same should apply to this problem.
A string like
.css("border", "1px 1px -1px 4px") would be unlikely to be used in repeating patterns, and it would be more likely that "border-left-width" would be changed. We can ignore the harder-to-catch and less-likely-to-be-invalid shorthand properties: border, border-TRBL, padding, border-width, font, outline, box-shadow.Instead, a simple one-liner (with a complex RE) can catch the others. Something like:
if (/^(border-(spacing|-(top|right|bottom|left)(-width)?|image-(slice|width|outset))|padding-(top|right|bottom|left)|font|(font|background)-size|((min|max|device)-)?(width|height)|(outline|column-rule)-width|line-height|widows|column-gap|color(-index)?|monochrome|marquee-play-count)$/.test(prop) && /^\\s*-/.test(val)) return $obj;
Heck, even the other ones aren't difficult to parse. I already included above the ones that feature the negative illegal on the front end.
else if (/^(border(-(top|right|bottom|left))?|padding|outline)$/.test(prop) && /(^|\\s+)-\\d+/.test(val)) return $obj;
That covers all of them except for box-shadow, which likely isn't going to be used, anyway.
Changed March 02, 2012 03:04AM UTC by comment:3
We should handle the properties from fxAttrs, and leave the rest to another plugin or the end user.
The implementation would be really small. We're already guarding against this for width and height, so we can just change:
jQuery.each([ "width", "height" ], function( i, prop ) {
to:
jQuery.each( fxAttrs.concat.apply( [], fxAttrs ), function( i, prop ) {
Changed March 02, 2012 03:32AM UTC by comment:4
| resolution: | → fixed |
|---|---|
| status: | new → closed |
Fix #11415: Stop non-negative prop undershoot on animation.
This doesn't fix *all* of them (see the ticket for a supposedly complete list) but these were already handy so it was relatively cheap to fix them. If you need others fixed, add a custom step function as was done here. Thanks @scott_gonzalez!
Changeset: 56426261f0ee97d6d2f903d08b137a6f5f2c0916
Changed March 02, 2012 04:12PM UTC by comment:5
| component: | unfiled → effects |
|---|---|
| milestone: | None → 1.7.2 |
| priority: | undecided → low |
| type: | bug → enhancement |
Changed March 17, 2012 01:25AM UTC by comment:6
Good, good. Thanks for your help on this from both JQ/UI sides.
As a thought exercise, if it were Chrome or Firefox that was picky would you want this behavior? Is IE wrong about the error or is the code wrong in a possibly-benign way and other browsers ignored it?
With
.css("border", "1px 1px -1px 4px"), are we supposed to parse the shorthand property and look for negative numbers? A more horrifying fix IMO would be to wrap it all in a try/catch and ignore any hint of an error coming out of the property assignment.I think our goals for cross-browser consistency should cover valid inputs, not invalid ones. Invalid inputs are errors and should be fixed.