[webkit-reviews] review granted: [Bug 109981] Convert residual-style.html test to a reftest (and fix typos) : [Attachment 188686] Patch

Sat Feb 16 12:51:30 PST 2013

Darin Adler <darin at apple.com> has granted Christian Biesinger
<cbiesinger at chromium.org>'s request for review:
Bug 109981: Convert residual-style.html test to a reftest (and fix typos)
https://bugs.webkit.org/show_bug.cgi?id=109981

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

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

> LayoutTests/fast/flexbox/box-orient-button.html:47
> -<button id="horizontal" clsas="box horizontal">
> +<button id="horizontal" class="box horizontal">

How can we land this change without updating test results?

> LayoutTests/fast/invalid/residual-style-expected.html:1
> +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN">

Why an old DOCTYPE? Why not an HTML5 DOCTYPE?

> LayoutTests/fast/invalid/residual-style-expected.html:5
> +.xfail { color: red }

Does "x" here stand for "expected"? If so, why abbreviate it to one letter?
Mysterious for people dealing with this test later. I’d name this
"expected-failure" since these are expected failures.

> LayoutTests/fast/invalid/residual-style-expected.html:12
> +This test case illustrates some residual style tag examples.  Unless
otherwise noted, the behavior should match Firefox.

Why the two spaces here after the period? These will collapse into one.

> LayoutTests/fast/invalid/residual-style-expected.html:18
> +<hr>
> +A: <font>All of this should be green</font>
> +

Why all the blank lines? I think this would be much easier to read if more of
it fit on the screen.