Bug 81869

Summary: Rename cssValueKeywordID and cssPropertyID to make it more obvious that they should only be called from within the parser.
Product: WebKit Reporter: Luke Macpherson <macpherson>
Component: CSSAssignee: Luke Macpherson <macpherson>
Status: RESOLVED WONTFIX    
Severity: Normal CC: eric, haraken, kling, koivisto, macpherson, menard, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch with updated ChangeLog. none

Luke Macpherson
Reported 2012-03-21 21:34:32 PDT
Rename cssValueKeywordID and cssPropertyID to make it more obvious that they should only be called from CSSGrammar.y
Attachments
Patch (5.24 KB, patch)
2012-03-21 21:36 PDT, Luke Macpherson 		no flags
DetailsFormatted DiffDiff
Patch with updated ChangeLog. (5.26 KB, patch)
2012-03-22 16:00 PDT, Luke Macpherson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Luke Macpherson
Comment 1 2012-03-21 21:36:36 PDT
Created attachment 133183 [details] Patch
Alexis Menard (darktears)
Comment 2 2012-03-22 04:07:12 PDT
Comment on attachment 133183 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133183&action=review > Source/WebCore/ChangeLog:3 > + Rename cssValueKeywordID and cssPropertyID to make it more obvious that they should only be called from CSSGrammar.y Well maybe the title is not correct. CSSParser is using it, so "they should only be called from CSSGrammar.y" is a bit incorrect. Maybe they should be used only by CSS parsing related classes (CSSGrammar.y and CSSParser).
Luke Macpherson
Comment 3 2012-03-22 16:00:10 PDT
Created attachment 133375 [details] Patch with updated ChangeLog.
Kentaro Hara
Comment 4 2012-04-10 03:04:15 PDT
Comment on attachment 133375 [details] Patch with updated ChangeLog. Makes sense.
Luke Macpherson
Comment 5 2012-05-06 21:19:08 PDT
This code change is no longer relevant, so marking bug resolved.
Darin Adler
Comment 6 2012-05-07 10:27:50 PDT
Comment on attachment 133375 [details] Patch with updated ChangeLog. View in context: https://bugs.webkit.org/attachment.cgi?id=133375&action=review > Source/WebCore/ChangeLog:11 > + Currently it is hard to tell which functions are only public to help CSSGrammar.y, and which are intended > + to be part of the public interface. This change aims to make that distinction slightly more obvious. Really? I think the type, CSSParserString, already made the fact that this is for use only in the CSS parser perfectly clear. Why so many refactoring patches that make no concrete improvement. This is unneeded code churn!
Luke Macpherson
Comment 7 2012-05-07 15:37:44 PDT
Yes, which is why I've already taken the r+ bit off and marked this bug as WONTFIX.
Note You need to log in before you can comment on or make changes to this bug.