Bug 77960

Summary: CSS3 calc() - simple parse time evaluation
Product: WebKit Reporter: Mike Lawther <mikelawther>
Component: New BugsAssignee: Mike Lawther <mikelawther>
Status: RESOLVED FIXED    
Severity: Normal CC: macpherson, menard, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 16662    
Attachments:
Description Flags
Patch
none
Patch for landing none

Mike Lawther
Reported 2012-02-07 03:45:24 PST
CSS3 calc() - simple parse time evaluation
Attachments
Patch (7.55 KB, patch)
2012-02-07 03:49 PST, Mike Lawther 		no flags
DetailsFormatted DiffDiff
Patch for landing (7.44 KB, patch)
2012-02-07 16:50 PST, Mike Lawther 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mike Lawther
Comment 1 2012-02-07 03:49:28 PST
Created attachment 125806 [details] Patch
Ojan Vafai
Comment 2 2012-02-07 10:07:58 PST
Comment on attachment 125806 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125806&action=review I know it's a pain to post these smaller patches, but these are 1000x easier to review. :) > Source/WebCore/css/CSSCalculationValue.cpp:117 > + > + ASSERT_NOT_REACHED(); > + return 0; Does the compiler complain if you leave this out? If not, I'd just delete this since you will get a warning if a new Calc type is added since your switch above doesn't have a "default" clause (== a good thing!). > Source/WebCore/css/CSSCalculationValue.cpp:206 > + case CalcMod: > + if (rightValue) > + return static_cast<int>(leftValue) % static_cast<int>(rightValue); > + return std::numeric_limits<double>::quiet_NaN(); Wasn't mod dropped from the spec? > Source/WebCore/css/CSSCalculationValue.cpp:209 > + ASSERT_NOT_REACHED(); > + return 0; Ditto re: deleting the unreachable code if the compiler doesn't complain.
Mike Lawther
Comment 3 2012-02-07 16:49:46 PST
Comment on attachment 125806 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125806&action=review >> Source/WebCore/css/CSSCalculationValue.cpp:117 >> + return 0; > > Does the compiler complain if you leave this out? If not, I'd just delete this since you will get a warning if a new Calc type is added since your switch above doesn't have a "default" clause (== a good thing!). Nope, it doesn't. Removed. >> Source/WebCore/css/CSSCalculationValue.cpp:206 >> + return std::numeric_limits<double>::quiet_NaN(); > > Wasn't mod dropped from the spec? Indeed it has been. I've removed the evaluation code, and I'll get rid of the enum value in another patch. >> Source/WebCore/css/CSSCalculationValue.cpp:209 >> + return 0; > > Ditto re: deleting the unreachable code if the compiler doesn't complain. Removed.
Mike Lawther
Comment 4 2012-02-07 16:50:31 PST
Created attachment 125956 [details] Patch for landing
WebKit Review Bot
Comment 5 2012-02-07 19:40:49 PST
Comment on attachment 125956 [details] Patch for landing Clearing flags on attachment: 125956 Committed r107030: <http://trac.webkit.org/changeset/107030>
WebKit Review Bot
Comment 6 2012-02-07 19:40:53 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.