[webkit-reviews] review canceled: [Bug 11645] Table with percentage column widths doesn't scale to fill the entire width of a table containing it : [Attachment 162957] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 2 17:45:01 PDT 2012


Julien Chaffraix <jchaffraix at webkit.org> has canceled mitz at webkit.org
<mitz at webkit.org>'s request for review:
Bug 11645: Table with percentage column widths doesn't scale to fill the entire
width of a table containing it
https://bugs.webkit.org/show_bug.cgi?id=11645

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

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=162957&action=review


The code change is fine, AFAICT the test is progressing but would like to see
another round with the proper justifications.

> LayoutTests/ChangeLog:14
> +	   * platform/chromium-linux/fast/table/025-expected.png:
> +	   * platform/chromium-win/fast/table/025-expected.png:

You should only need the chromium-linux baseline here. The fact that it fails
leads me to think that your machine's configuration doesn't match the EWS.
Unfortunately, they don't upload their baselines to the bugs anymore and I
don't know if those are available somewhere.

> LayoutTests/ChangeLog:16
> +	   Existing test case's results have been modified as per this change.

As pointed out by Dan, you should explain what the difference is and why this
is a progression.

As written, this line answers none of those questions and thus adds no value to
the reader.

> LayoutTests/fast/table/scale-nested-percent-width-cols-expected.html:13
> +			   <table style="width: 100%; background: green;">

The difference between the expected and the normal result is really not clear
cut which makes it hard to say if it's correct.

Also if you used to not scale when you encountered any percent width, the test
as written would pass while actually failing. The expected result should
probably have fixed lengths to avoid any such ambiguity.


More information about the webkit-reviews mailing list