[webkit-reviews] review granted: [Bug 137171] Add support for midpoint to CSS gradients : [Attachment 238995] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 1 07:36:37 PDT 2014


Darin Adler <darin at apple.com> has granted Rik Cabanier <cabanier at adobe.com>'s
request for review:
Bug 137171: Add support for midpoint to CSS gradients
https://bugs.webkit.org/show_bug.cgi?id=137171

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=238995&action=review


> Source/WebCore/css/CSSGradientValue.cpp:116
> +    for (unsigned i = 0; i < m_stops.size(); i++) {
> +	   if (!m_stops[i].isMidpoint &&
styleResolver->colorFromPrimitiveValueIsDerivedFromElement(m_stops[i].m_color.g
et())) {
>	       m_stops[i].m_colorIsDerivedFromElement = true;
>	       derived = true;
>	       break;
>	   }
> +    }

Should come back here after this patch and change this to:

    for (auto& stop : m_stops)

This would read more clearly written that way.

> Source/WebCore/css/CSSGradientValue.cpp:133
> +    for (unsigned i = 0; i < result->m_stops.size(); i++) {
> +	   if (!result->m_stops[i].isMidpoint)
> +	       result->m_stops[i].m_resolvedColor =
styleResolver->colorFromPrimitiveValue(result->m_stops[i].m_color.get());
> +    }

Should come back here after this patch and change this to:

    for (auto& stop : result->m_stops)

This would read more clearly written that way.

> Source/WebCore/css/CSSGradientValue.cpp:279
> +    // a line in that region. We then add 5 more color stops on that side to
minimize the change
> +    // how the luminance changes at each of the colorstops. We don't have to
add as many on the other side

"minimize the change how the luminance changes"?

"colorstops" -> "color stops"


More information about the webkit-reviews mailing list