Bug 98557

Summary: Add a "non-zero" flag for CSSParser::validUnit()
Product: WebKit Reporter: Tab Atkins <tabatkins>
Component: CSSAssignee: Tab Atkins <tabatkins>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: bfulgham, cmarcelo, macpherson, menard, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 65159    
Attachments:
Description Flags
Patch
none
Patch none

Tab Atkins
Reported 2012-10-05 14:32:08 PDT
CSSParser::validUnit() can currently take a "non-negative" flag, but some properties further need to be strictly positive. For example, "column-count" only accepts integers > 0, but currently we accept "0" as a valid value (see bug 65159). This patch adds a non-zero flag to the flag enum that validUnit() uses, so this can be easily checked.
Attachments
Patch (2.16 KB, patch)
2012-10-05 14:43 PDT, Tab Atkins 		no flags
DetailsFormatted DiffDiff
Patch (2.10 KB, patch)
2012-10-05 19:11 PDT, Tab Atkins 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tab Atkins
Comment 1 2012-10-05 14:43:40 PDT
Created attachment 167384 [details] Patch
Tab Atkins
Comment 2 2012-10-05 19:11:37 PDT
Created attachment 167435 [details] Patch
Tab Atkins
Comment 3 2012-10-05 19:13:49 PDT
Whoops, sorry for the noise. This patch is identical to the previous. I hadn't refreshed the bug page since before I uploaded the first patch, so I didn't realize it was already committed. :/ Anyway, r+ and cq+ please. ^_^
Eric Seidel (no email)
Comment 4 2012-10-08 11:52:11 PDT
Comment on attachment 167435 [details] Patch It seems like for such a simple change we might want to just roll this into bug 65159. I also probably would have called this FPositive since non-zero could be negative, no?
Tab Atkins
Comment 5 2012-10-08 12:11:15 PDT
Okay, I wasn't sure what the protocol was for this kind of thing. I went with NonZero rather than Positive because it might be useful to have non-zero behavior specifically. However, I don't know of any case where that's true, so I might as well just rename it and shorten its usage. Okay, so I'll fold this into bug 65159.
Tab Atkins
Comment 6 2012-10-08 12:56:32 PDT
Comment on attachment 167435 [details] Patch Turning off cq? to see if this gets submitted as part of bug 65159 instead. If so, I'll just cancel this bug out entirely.
Brent Fulgham
Comment 7 2022-08-02 13:28:27 PDT
Comment on attachment 167435 [details] Patch This code is no longer relevant.
Note You need to log in before you can comment on or make changes to this bug.