[webkit-reviews] review granted: [Bug 85348] REGRESSION: Focused TR element draws its focus outline in the wrong place : [Attachment 155031] Patch

Fri Jul 27 13:27:50 PDT 2012

Julien Chaffraix <jchaffraix at webkit.org> has granted Pravin D
<pravind.2k4 at gmail.com>'s request for review:
Bug 85348: REGRESSION: Focused TR element draws its focus outline in the wrong
place
https://bugs.webkit.org/show_bug.cgi?id=85348

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

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

> LayoutTests/ChangeLog:8
> +	   The focus outline is always drawn around the first row of the table
section irrespective of the row in focus.

The tense is wrong. It used to be the case but it was solved by the bug.

> LayoutTests/ChangeLog:10
> +	   Adding regression test cases.

There is one test case so it should be "Adding a regression test case."

> LayoutTests/fast/table/table-row-focus-outline-paint-expected.txt:1
> +layer at (0,0) size 800x600

We don't care about the text dump so this test should be dumpAsText(true);

> LayoutTests/fast/table/table-row-focus-outline-paint.html:1
> +<!DOCTYPE html>

Nit: Maybe add the missing <html>.

> LayoutTests/fast/table/table-row-focus-outline-paint.html:4
> +Issue: The focus outline is always drawn around the first row irrespective
of the current focused row.

We call this a focus ring, let's keep the naming consistent so rename all the
focus outline into focus ring (including the test case name).

> LayoutTests/fast/table/table-row-focus-outline-paint.html:5
> +About TestCase: The test case checks if the focus outline is drawn around
the table row which currently has focus.

s/About TestCase/Description/

> LayoutTests/fast/table/table-row-focus-outline-paint.html:16
> +<table >

trailing space.