Bug 81869 – Rename cssValueKeywordID and cssPropertyID to make it more obvious that they should only be called from within the parser.

• Status: RESOLVED
• Resolution: WONTFIX
• Component: CSS

[Luke Macpherson]

Rename cssValueKeywordID and cssPropertyID to make it more obvious that they should only be called from CSSGrammar.y

[Luke Macpherson]

Created attachment 133183 [details]
Patch

[Alexis Menard (darktears)]

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]

Created attachment 133375 [details]
Patch with updated ChangeLog.

[Kentaro Hara]

Comment on attachment 133375 [details]
Patch with updated ChangeLog.

Makes sense.

[Luke Macpherson]

This code change is no longer relevant, so marking bug resolved.

[Darin Adler]

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]

Yes, which is why I've already taken the r+ bit off and marked this bug as WONTFIX.