[webkit-reviews] review granted: [Bug 210017] 'currentcolor' doesn't need setHasExplicitlyInheritedProperties marking anymore : [Attachment 395579] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 210017: 'currentcolor' doesn't need setHasExplicitlyInheritedProperties
marking anymore
https://bugs.webkit.org/show_bug.cgi?id=210017

Attachment 395579: patch

https://bugs.webkit.org/attachment.cgi?id=395579&action=review




--- Comment #6 from Darin Adler <darin at apple.com> ---
Comment on attachment 395579
  --> https://bugs.webkit.org/attachment.cgi?id=395579
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395579&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2169
> +	       // While most color properties are not inherited the value
'currentcolor' resolves to the value of the inherited 'color' property.

Needs a comma after the word "inherited".

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2200
> +	       for (size_t i = 0; i < shorthand.length(); ++i) {
> +		   if (!hasValidStyleForProperty(element,
shorthand.properties()[i]))
> +		       return false;
> +	       }

Seems like StylePropertyShorthand could use a way to use a for loop without
writing it out. Since length() returns unsigned it’s not clear it’s helpful to
use size_t for the loop, but that question only arises because we can’t use a
modern for loop.

Wish the standard library offered a way to return a pointer and length as a
for-loop-compatible range. I’m sure it’s coming in C++20.