You can demonstrate with this example: the second TR has focus, but the focus outline is drawn around the first row. <table> <tr> <td>First row</td> </tr> <tr tabIndex=0 id="row2"> <td>Second row</td> </tr> </table> <script> document.getElementById('row2').focus(); </script> You can confirm (by printing document.activeElement) that focus is on the second row, but the outline is drawn in the wrong place. This is definitely a regression;.
Could you please attach a full working test case? I don't get any focus ring at all for this in Safari 5.1.5.
Created attachment 154765 [details] Test case: If you tab several times, you should see that the focus ring is always drawn on the first row We got a similar report on Chromium, attaching the test case as I think it's relevant.
Seems like our focus ring logic doesn't add the row offset from the table: thus it is drawn on top of the first row instead of the right row. I believe the fix to be simple and is about adding the location to the focus ring's position in RenderBox::addFocusRingRects. Due to RenderBox being used for a lot of renderers, it is likely impacting other cases.
The issue as pointed out by Julien was due to wrong paint offset for focus rings. The issue has been fixed by the changeset http://trac.webkit.org/changeset/123793 RenderBox::addFocusRingRects() is called from RenderObject::paintFocusRing() which is called from RenderObject::paintOutline(). The fix for the bug https://bugs.webkit.org/show_bug.cgi?id=92389 fixes the paint offset for row outlines which is the same offset which sent to draw focus rings... Also the issue is not repo on r123859 (Qt linux port)
> RenderBox::addFocusRingRects() is called from RenderObject::paintFocusRing() which is called from RenderObject::paintOutline(). The fix for the bug https://bugs.webkit.org/show_bug.cgi?id=92389 fixes the paint offset for row outlines which is the same offset which sent to draw focus rings... Good analysis, this bug is related with bug 92389. However I wouldn't say they are duplicated as one is about the outline, this one is about the focus ring. It would be nice to land the test case to ensure we don't regress this case.
Created attachment 155031 [details] Patch
Comment on attachment 155031 [details] Patch 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.
Created attachment 155047 [details] Patch
Created attachment 155050 [details] Patch
Comment on attachment 155050 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155050&action=review > LayoutTests/fast/table/table-row-focus-ring-paint.html:13 > +if(window.testRunner) > + testRunner.dumpAsText(true); You could also have pushed that to the existing <script>.
Comment on attachment 155050 [details] Patch Attachment 155050 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13367874 New failing tests: fast/table/table-row-focus-ring-paint.html
Created attachment 155085 [details] Archive of layout-test-results from gce-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 155166 [details] Patch
Comment on attachment 155166 [details] Patch Attachment 155166 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13395041 New failing tests: fast/table/table-row-focus-ring-paint.html
Created attachment 155169 [details] Archive of layout-test-results from gce-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 155171 [details] Patch
Comment on attachment 155171 [details] Patch Clearing flags on attachment: 155171 Committed r124047: <http://trac.webkit.org/changeset/124047>
All reviewed patches have been landed. Closing bug.