[webkit-reviews] review granted: [Bug 67166] Update CSS3 gradient support to the latest spec version : [Attachment 181960] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 9 11:43:16 PST 2013


Simon Fraser (smfr) <simon.fraser at apple.com> has granted Tab Atkins
<tabatkins at google.com>'s request for review:
Bug 67166: Update CSS3 gradient support to the latest spec version
https://bugs.webkit.org/show_bug.cgi?id=67166

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

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


> Source/WebCore/css/CSSParser.cpp:7380
> +	   if ((location = valueFromSideKeyword(a, isHorizontal))) {
> +	       if (isHorizontal)
> +		   endX = location;
> +	       else
> +		   endY = location;
> +
> +	       a = args->next();
> +	       if (!a)
> +		   return false;
> +
> +	       if ((location = valueFromSideKeyword(a, isHorizontal))) {
> +		   if (isHorizontal) {
> +		       if (endX)
> +			   return false;
> +		       endX = location;
> +		   } else {
> +		       if (endY)
> +			   return false;
> +		       endY = location;
> +		   }
> +
> +		   args->next();
> +	       }
> +
> +	       expectComma = true;
> +	   } else
> +	       return false;

I think this would be better as
location = valueFromSideKeyword(a, isHorizontal);
if (!location)
  return false

You should keep non-exceptional code outside big if statements if possible.

> LayoutTests/ChangeLog:48
> +	   * fast/gradients/unprefixed-color-stop-units-expected.txt: Added.
> +	   * fast/gradients/unprefixed-color-stop-units.html: Added.
> +	   * fast/gradients/unprefixed-color-stops-expected.txt: Added.
> +	   * fast/gradients/unprefixed-color-stops.html: Added.
> +	   * fast/gradients/unprefixed-generated-gradients-expected.txt: Added.

> +	   * fast/gradients/unprefixed-generated-gradients.html: Added.
> +	   * fast/gradients/unprefixed-gradient-parsing-expected.txt: Added.
> +	   * fast/gradients/unprefixed-gradient-parsing.html: Added.
> +	   * fast/gradients/unprefixed-linear-angle-gradients-expected.txt:
Added.
> +	   * fast/gradients/unprefixed-linear-angle-gradients.html: Added.
> +	   *
fast/gradients/unprefixed-linear-right-angle-gradients-expected.txt: Added.
> +	   * fast/gradients/unprefixed-linear-right-angle-gradients.html:
Added.
> +	   * fast/gradients/unprefixed-list-item-gradient-expected.txt: Added.
> +	   * fast/gradients/unprefixed-list-item-gradient.html: Added.
> +	   * fast/gradients/unprefixed-radial-gradients-expected.txt: Added.
> +	   * fast/gradients/unprefixed-radial-gradients.html: Added.
> +	   * fast/gradients/unprefixed-radial-gradients2-expected.txt: Added.
> +	   * fast/gradients/unprefixed-radial-gradients2.html: Added.
> +	   * fast/gradients/unprefixed-radial-gradients3-expected.txt: Added.
> +	   * fast/gradients/unprefixed-radial-gradients3.html: Added.
> +	   * fast/gradients/unprefixed-repeating-end-fill-expected.txt: Added.
> +	   * fast/gradients/unprefixed-repeating-end-fill.html: Added.
> +	   * fast/gradients/unprefixed-repeating-linear-gradient-expected.txt:
Added.
> +	   * fast/gradients/unprefixed-repeating-linear-gradient.html: Added.
> +	   * fast/gradients/unprefixed-repeating-radial-gradients-expected.txt:
Added.
> +	   * fast/gradients/unprefixed-repeating-radial-gradients.html: Added.
> +	   *
fast/gradients/unprefixed-zero-range-repeating-gradient-hang-expected.txt:
Added.
> +	   * fast/gradients/unprefixed-zero-range-repeating-gradient-hang.html:
Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-color-stop-units-expected.png
: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-color-stops-expected.png:
Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-generated-gradients-expected.
png: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-linear-angle-gradients-expect
ed.png: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-linear-right-angle-gradients-
expected.png: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-list-item-gradient-expected.p
ng: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-radial-gradients-expected.png
: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-radial-gradients2-expected.pn
g: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-radial-gradients3-expected.pn
g: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-repeating-end-fill-expected.p
ng: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-repeating-linear-gradient-exp
ected.png: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-repeating-radial-gradients-ex
pected.png: Added.
> +	   *
platform/chromium-linux/fast/gradients/unprefixed-zero-range-repeating-gradient
-hang-expected.png: Added.

You should edit the TestExpectation files for non-Chromium platforms to
indicate that image results are missing for the tests that expect them.


More information about the webkit-reviews mailing list