[webkit-reviews] review granted: [Bug 38906] Expose CSS rule body start/end offsets in the parent stylesheet : [Attachment 57031] [PATCH] pfeldman's comments addressed, inspectorStyleSheet bug fixed

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 25 14:27:08 PDT 2010


Pavel Feldman <pfeldman at chromium.org> has granted Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 38906: Expose CSS rule body start/end offsets in the parent stylesheet
https://bugs.webkit.org/show_bug.cgi?id=38906

Attachment 57031: [PATCH] pfeldman's comments addressed, inspectorStyleSheet
bug fixed
https://bugs.webkit.org/attachment.cgi?id=57031&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
WebCore/css/CSSGrammar.y:893
 +	    p->markEnd();
Nit: I would call markEnd and reset Marks from within createStyleRule just to
minimize the coupling here (just as you do call markStart from within
updateLastSelector*).

WebCore/css/CSSParser.cpp:5500
 +  void CSSParser::markStart()
nit: markRuleBodyStart?

WebCore/css/CSSParser.cpp:5507
 +  void CSSParser::markEnd()
markRuleBodyEnd

WebCore/css/CSSParser.h:246
 +	    void resetMarks() { m_startOffset = m_endOffset = 0; }
resetRuleBodyMarks ?

WebCore/css/CSSParser.h:241
 +	    unsigned m_startOffset;
m_ruleBodyStartOffset

WebCore/css/CSSParser.h:242
 +	    unsigned m_endOffset;
m_ruleBodyEndOffset


Please land it with comments addressed. There is no need to ask for another
review. Reviewing same changes over and over after you've got r+ is painful. It
is better to land as is and get a follow-up change reviewed in these cases.


More information about the webkit-reviews mailing list