2011-02-17 Ojan Vafai <ojan@chromium.org>

        Reviewed by Adam Barth.

        use actual browser focus in the code review tool
        https://bugs.webkit.org/show_bug.cgi?id=54726

        This makes keyboard handling play better with focusable element
        (i.e. links/textareas/buttons). Also, in theory, this makes
        the review tool more amenable to screen readers (I think).

        * PrettyPatch/PrettyPatch.rb:
        * code-review.js:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@79163 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Websites/bugs.webkit.org/ChangeLog b/Websites/bugs.webkit.org/ChangeLog
index c8f89291c98476b66ad3a6cf426bcd01a0e79ddc..e8ec48ef32e8b749a37eb96a76dc6ecea4749b7e 100644 (file)
--- a/Websites/bugs.webkit.org/ChangeLog
+++ b/Websites/bugs.webkit.org/ChangeLog
@@ -1,3 +1,17 @@
+2011-02-17  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        use actual browser focus in the code review tool
+        https://bugs.webkit.org/show_bug.cgi?id=54726
+
+        This makes keyboard handling play better with focusable element
+        (i.e. links/textareas/buttons). Also, in theory, this makes
+        the review tool more amenable to screen readers (I think).
+
+        * PrettyPatch/PrettyPatch.rb:
+        * code-review.js:
+
 2011-02-17  Ojan Vafai  <ojan@chromium.org>
 
         Reviewed by Antonio Gomes.

diff --git a/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb b/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
index c33054e7007689a6881b6ec2a781e7109ea45529..1718268b12a4c9c2551177f91e5a9f8aad7f84c3 100644 (file)
--- a/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
+++ b/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
@@ -399,7 +399,7 @@ body {
   width: 6em;
 }
 
-.focused {
+div:focus {
   outline: 1px solid blue;
   outline-offset: -1px;
 }

diff --git a/Websites/bugs.webkit.org/code-review.js b/Websites/bugs.webkit.org/code-review.js
index 9386493a0c5c8430d8b610c152f1f938644bf026..b92000bc1b47af034a03d6d75eb8f8923cae28cd 100644 (file)
--- a/Websites/bugs.webkit.org/code-review.js
+++ b/Websites/bugs.webkit.org/code-review.js
@@ -1074,9 +1074,6 @@ var CODE_REVIEW_UNITTEST;
 
     updateToolbarAnchorState();
     loadDiffState();
-
-    // Ensure the body has focus so that it receives key events.
-    document.body.focus();
   });
 
   function handleReviewFormLoad() {
@@ -1337,20 +1334,23 @@ var CODE_REVIEW_UNITTEST;
   }
 
   function focusOn(node) {
-    // FIXME: We should probably give propery browser focus here.
-    $('.focused').removeClass('focused');
     if (node.length == 0)
       return;
-    $(document).scrollTop(node.addClass('focused').position().top - window.innerHeight / 2);
+
+    // Give a tabindex so the element can receive actual browser focus.
+    // -1 makes the element focusable without actually putting in in the tab order.
+    node.attr('tabindex', -1);
+    node.focus();
+    $(document).scrollTop(node.position().top - window.innerHeight / 2);
   }
 
   function focusNext(filter, direction) {
-    var focusable_nodes = $('.frozenComment,.previousComment,.DiffBlock,.overallComments').filter(function() {
+    var focusable_nodes = $('a,.frozenComment,.previousComment,.DiffBlock,.overallComments').filter(function() {
       return !$(this).hasClass('DiffBlock') || $('.add,.remove', this).size();
     });
 
     var is_backward = direction == DIRECTION.BACKWARD;
-    var index = focusable_nodes.index($('.focused'));
+    var index = focusable_nodes.index($(document.activeElement));
     if (index == -1 && is_backward)
       index = focusable_nodes.length;
 
@@ -1432,9 +1432,10 @@ var CODE_REVIEW_UNITTEST;
   }
   
   function handleEnterKeyPress() {
-    var focused = $('.focused');
-    if (!focused.size())
-      return false;
+    if (document.activeElement.nodeName == 'BODY')
+      return;
+
+    var focused = $(document.activeElement);
 
     if (focused.hasClass('frozenComment')) {
       unfreezeComment(focused);