[webkit-reviews] review denied: [Bug 80191] Convert some fast/regions pixel tests to reftests : [Attachment 130402] Patch for the bug

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 12 15:03:18 PDT 2012


Dirk Schulze <krit at webkit.org> has denied Rebecca Hauck <rhauck at adobe.com>'s
request for review:
Bug 80191: Convert some fast/regions pixel tests to reftests
https://bugs.webkit.org/show_bug.cgi?id=80191

Attachment 130402: Patch for the bug
https://bugs.webkit.org/attachment.cgi?id=130402&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=130402&action=review


> LayoutTests/ChangeLog:6
> +	   Convert some fast/regions pixel tests to reftests
> +	   https://bugs.webkit.org/show_bug.cgi?id=80191
> +
> +	   Reviewed by NOBODY (OOPS!).

Your change log should be more explicit what you try to do with your patch, why
you do it and how you do it. Also, you modified the test itself without a
comment why you did it. Is the test still testing the same?

> LayoutTests/fast/regions/bottom-overflow-out-of-first-region.html:-6
> -	   text-align: justify;

Why did you remove this line?

> LayoutTests/fast/regions/bottom-overflow-out-of-first-region.html:-61
> -    <div id="region3"></div>

It is not desirable to modify the test to match the reference. It should be the
other way around. Why did you remove the region here?


More information about the webkit-reviews mailing list