Bug 129562

Summary: No need to ASSERT on infinite values in CSS lengths
Product: WebKit Reporter: Dean Jackson <dino>
Component: CSSAssignee: Dean Jackson <dino>
Status: NEW ---    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, gyuyoung.kim, macpherson, menard
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

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.