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+

Dean Jackson
Reported 2014-03-01 16:01:29 PST
Infinite values should be clamped to the maximum allowed css lengths.
Attachments
Patch (6.61 KB, patch)
2014-03-01 16:31 PST, Dean Jackson 		darin: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dean Jackson
Comment 1 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
Dean Jackson
Comment 2 2014-03-01 16:31:23 PST
Created attachment 225573 [details] Patch
Darin Adler
Comment 3 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));
Darin Adler
Comment 4 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!
Dean Jackson
Comment 5 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).
Dean Jackson
Comment 6 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.
Note You need to log in before you can comment on or make changes to this bug.