Bug 129562 - No need to ASSERT on infinite values in CSS lengths
Summary: No need to ASSERT on infinite values in CSS lengths
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-01 16:01 PST by Dean Jackson
Modified: 2014-03-02 14:05 PST (History)
6 users (show)

See Also:

Attachments
Patch (6.61 KB, patch)
2014-03-01 16:31 PST, Dean Jackson 		darin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2014-03-01 16:01:29 PST 
Infinite values should be clamped to the maximum allowed css lengths.
Comment 1 Dean Jackson 2014-03-01 16:25:46 PST 
Actually, they are already clamped. There is no need to assert. It's valid to say
left: 1379382749871309473209874092874098723px
Comment 2 Dean Jackson 2014-03-01 16:31:23 PST 
Created attachment 225573 [details]
Patch
Comment 3 Darin Adler 2014-03-01 16:39:02 PST 
Comment on attachment 225573 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=225573&action=review

> Source/WebCore/css/CSSPrimitiveValue.cpp:-259
> -    ASSERT(std::isfinite(num));

std::isfinite also checks for NAN. Instead of deleting the assertion I think you should change it to:

    ASSERT(!std::isnan(num));
Comment 4 Darin Adler 2014-03-01 16:39:54 PST 
Comment on attachment 225573 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=225573&action=review

> Source/WebCore/ChangeLog:12
> +        Tests: fast/css/box-shadow-invalid-value.html
> +               fast/css/length-infinite-value.html

Sure seems like there are a lot more cases to test!
Comment 5 Dean Jackson 2014-03-01 16:58:54 PST 
(In reply to comment #4)
> (From update of attachment 225573 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=225573&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        Tests: fast/css/box-shadow-invalid-value.html
> > +               fast/css/length-infinite-value.html
> 
> Sure seems like there are a lot more cases to test!

Yeah. I'll exercise anything that would create a primitive value. I guess I should probably try more properties too (especially those that take combinations of values - we hit this in shadow parsing but it really applies to everything).
Comment 6 Dean Jackson 2014-03-02 14:05:25 PST 
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 225573 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=225573&action=review
> > 
> > > Source/WebCore/ChangeLog:12
> > > +        Tests: fast/css/box-shadow-invalid-value.html
> > > +               fast/css/length-infinite-value.html
> > 
> > Sure seems like there are a lot more cases to test!
> 
> Yeah. I'll exercise anything that would create a primitive value. I guess I should probably try more properties too (especially those that take combinations of values - we hit this in shadow parsing but it really applies to everything).

Lucky you pushed me on this. I found other places where we barf.