[webkit-reviews] review granted: [Bug 134301] Implement parsing for CSS scroll snap points : [Attachment 235954] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 134301: Implement parsing for CSS scroll snap points
https://bugs.webkit.org/show_bug.cgi?id=134301

Attachment 235954: Patch
https://bugs.webkit.org/attachment.cgi?id=235954&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=235954&action=review


> Source/WebCore/css/StyleResolver.cpp:2869
> +	   } else {

You could return before this line to make the logic easier to follow.

> Source/WebCore/css/StyleResolver.cpp:2877
> +	       for (CSSValueListIterator i(value); i.hasMore(); i.advance()) {

i -> it

> Source/WebCore/rendering/style/RenderStyle.h:1627
> +    void setScrollSnapType(ScrollSnapType t) { SET_VAR(rareNonInheritedData,
m_scrollSnapType, t); }
> +    void setScrollSnapOffsetsX(Vector<Length>& t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, offsetsX, t); }
> +    void setScrollSnapOffsetsY(Vector<Length>& t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, offsetsY, t); }
> +    void setScrollSnapRepeatOffsetX(Length t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, repeatOffsetX, t); }

> +    void setScrollSnapRepeatOffsetY(Length t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, repeatOffsetY, t); }

> +    void setScrollSnapHasRepeatX(bool t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, hasRepeatX, t); }
> +    void setScrollSnapHasRepeatY(bool t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, hasRepeatY, t); }
> +    void setScrollSnapDestinationX(Length t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, destinationX, t); }
> +    void setScrollSnapDestinationY(Length t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, destinationY, t); }
> +    void setScrollSnapCoordinates(Vector<SnapCoordinate>& t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, coordinates, t); }
> +    void setScrollSnapUsesElementsX(bool t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, usesElementsX, t); }

> +    void setScrollSnapUsesElementsY(bool t) {
SET_VAR(rareNonInheritedData.access()->m_scrollSnapPoints, usesElementsY, t); }


Please use readable parameters names, rather than 't'.