Tweak results.html keyboard handling and flagging UI

https://bugs.webkit.org/show_bug.cgi?id=76331

Reviewed by Simon Fraser.

-Make it so that hitting j/k at the start selects the first/last test
instead of the second/none'th test.
-Use the unicode black flag character instead of " (flagged)"
-Make the text color of the whole selected row red. This showed
which row was selected without being to garish I think.
-Allow for having the flagged tests be space-separated instead of
newline separated.

* fast/harness/resources/results-test.js:
* fast/harness/results-expected.txt:
* fast/harness/results.html:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 2977c8d..1256739 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,22 @@
+2012-01-17  Ojan Vafai  <ojan@chromium.org>
+
+        Tweak results.html keyboard handling and flagging UI
+        https://bugs.webkit.org/show_bug.cgi?id=76331
+
+        Reviewed by Simon Fraser.
+
+        -Make it so that hitting j/k at the start selects the first/last test
+        instead of the second/none'th test.
+        -Use the unicode black flag character instead of " (flagged)"
+        -Make the text color of the whole selected row red. This showed
+        which row was selected without being to garish I think.
+        -Allow for having the flagged tests be space-separated instead of
+        newline separated.
+
+        * fast/harness/resources/results-test.js:
+        * fast/harness/results-expected.txt:
+        * fast/harness/results.html:
+
 2012-01-17  Simon Fraser  <simon.fraser@apple.com>
 
         Reduce the number of compositing test failures

diff --git a/LayoutTests/fast/harness/resources/results-test.js b/LayoutTests/fast/harness/resources/results-test.js
index 29a5bb9..5615ebf 100644 (file)
--- a/LayoutTests/fast/harness/resources/results-test.js
+++ b/LayoutTests/fast/harness/resources/results-test.js
@@ -564,50 +564,123 @@ function runTests()
     subtree['bar.html'] = mockExpectation('TEXT', 'FAIL');
     subtree['bar1.html'] = mockExpectation('TEXT', 'FAIL');
     subtree['bar2.html'] = mockExpectation('TEXT', 'FAIL');
-    
+
+    runTest(results, function() {
+        if (window.eventSender) {
+            eventSender.keyDown('k'); // previous
+            var testRows = document.querySelectorAll('#results-table tbody tr');
+            assertTrue(!testRows[0].classList.contains('current'));
+            assertTrue(!testRows[1].classList.contains('current'));
+            assertTrue(testRows[2].classList.contains('current'));
+        }
+    });
+
+    runTest(results, function() {
+        if (window.eventSender) {
+            eventSender.keyDown('j'); // next
+            var testRows = document.querySelectorAll('#results-table tbody tr');
+            assertTrue(testRows[0].classList.contains('current'));
+            assertTrue(!testRows[1].classList.contains('current'));
+            assertTrue(!testRows[2].classList.contains('current'));
+        }
+    });
+
     runTest(results, function() {
         assertTrue(document.getElementById('results-table'));
         assertTrue(visibleExpandLinks().length == 3);
         
         if (window.eventSender) {
+            eventSender.keyDown('i', ["metaKey"]);
+            eventSender.keyDown('i', ["shiftKey"]);
+            eventSender.keyDown('i', ["ctrlKey"]);
+            var testRows = document.querySelectorAll('#results-table tbody tr');
+            assertTrue(!testRows[0].classList.contains('current'));
+            assertTrue(!testRows[1].classList.contains('current'));
+            assertTrue(!testRows[2].classList.contains('current'));
+
             eventSender.keyDown('i'); // first
-            var expandButtons = document.querySelectorAll('#results-table tbody .expand-button');
-            assertTrue(expandButtons[0].classList.contains('current'));
-            assertTrue(!expandButtons[1].classList.contains('current'));
-            assertTrue(!expandButtons[2].classList.contains('current'));
+            assertTrue(testRows[0].classList.contains('current'));
+            assertTrue(!testRows[1].classList.contains('current'));
+            assertTrue(!testRows[2].classList.contains('current'));
+
+            eventSender.keyDown('j', ["metaKey"]);
+            eventSender.keyDown('j', ["shiftKey"]);
+            eventSender.keyDown('j', ["ctrlKey"]);
+            assertTrue(testRows[0].classList.contains('current'));
+            assertTrue(!testRows[1].classList.contains('current'));
+            assertTrue(!testRows[2].classList.contains('current'));
 
             eventSender.keyDown('j'); // next
-            var expandButtons = document.querySelectorAll('#results-table tbody .expand-button');
-            assertTrue(!expandButtons[0].classList.contains('current'));
-            assertTrue(expandButtons[1].classList.contains('current'));
-            assertTrue(!expandButtons[2].classList.contains('current'));
+            assertTrue(!testRows[0].classList.contains('current'));
+            assertTrue(testRows[1].classList.contains('current'));
+            assertTrue(!testRows[2].classList.contains('current'));
+
+            eventSender.keyDown('k', ["metaKey"]);
+            eventSender.keyDown('k', ["shiftKey"]);
+            eventSender.keyDown('k', ["ctrlKey"]);
+            assertTrue(!testRows[0].classList.contains('current'));
+            assertTrue(testRows[1].classList.contains('current'));
+            assertTrue(!testRows[2].classList.contains('current'));
 
             eventSender.keyDown('k'); // previous
-            var expandButtons = document.querySelectorAll('#results-table tbody .expand-button');
-            assertTrue(expandButtons[0].classList.contains('current'));
-            assertTrue(!expandButtons[1].classList.contains('current'));
-            assertTrue(!expandButtons[2].classList.contains('current'));
+            assertTrue(testRows[0].classList.contains('current'));
+            assertTrue(!testRows[1].classList.contains('current'));
+            assertTrue(!testRows[2].classList.contains('current'));
+
+            eventSender.keyDown('l', ["metaKey"]);
+            eventSender.keyDown('l', ["shiftKey"]);
+            eventSender.keyDown('l', ["ctrlKey"]);
+            assertTrue(testRows[0].classList.contains('current'));
+            assertTrue(!testRows[1].classList.contains('current'));
+            assertTrue(!testRows[2].classList.contains('current'));
 
             eventSender.keyDown('l'); // last
-            var expandButtons = document.querySelectorAll('#results-table tbody .expand-button');
-            assertTrue(!expandButtons[0].classList.contains('current'));
-            assertTrue(!expandButtons[1].classList.contains('current'));
-            assertTrue(expandButtons[2].classList.contains('current'));
+            assertTrue(!testRows[0].classList.contains('current'));
+            assertTrue(!testRows[1].classList.contains('current'));
+            assertTrue(testRows[2].classList.contains('current'));
 
             eventSender.keyDown('i'); // first
-            eventSender.keyDown('e'); // expand
+
+            eventSender.keyDown('e', ["metaKey"]);
+            eventSender.keyDown('e', ["shiftKey"]);
+            eventSender.keyDown('e', ["ctrlKey"]);
             var expandLinks = document.querySelectorAll('.expand-button-text');
+            assertTrue(!isExpanded(expandLinks[0]));
+
+            eventSender.keyDown('e'); // expand
+            assertTrue(isExpanded(expandLinks[0]));
+
+            eventSender.keyDown('c', ["metaKey"]);
+            eventSender.keyDown('c', ["shiftKey"]);
+            eventSender.keyDown('c', ["ctrlKey"]);
             assertTrue(isExpanded(expandLinks[0]));
 
             eventSender.keyDown('c'); // collapse
             assertTrue(isCollapsed(expandLinks[0]));
 
-            eventSender.keyDown('f'); // flag
+            eventSender.keyDown('f', ["metaKey"]);
+            eventSender.keyDown('f', ["shiftKey"]);
+            eventSender.keyDown('f', ["ctrlKey"]);
             var flaggedTestsTextbox = document.getElementById('flagged-tests');
-            flaggedTestsTextbox.innerText == 'bar.html';
+            assertTrue(flaggedTestsTextbox.innerText == '');
+
+            eventSender.keyDown('f'); // flag
+            assertTrue(flaggedTestsTextbox.innerText == 'foo/bar.html');
+
+            eventSender.keyDown('j'); // next
+            eventSender.keyDown('f'); // flag
+            assertTrue(flaggedTestsTextbox.innerText == 'foo/bar.html\nfoo/bar1.html');
+
+            document.getElementById('use-newlines').checked = false;
+            TestNavigator.updateFlaggedTests();
+            assertTrue(flaggedTestsTextbox.innerText == 'foo/bar.html foo/bar1.html');
 
             eventSender.keyDown('f'); // unflag
-            flaggedTestsTextbox.innerText == '';
+            assertTrue(flaggedTestsTextbox.innerText == 'foo/bar.html');
+
+            eventSender.keyDown('k'); // previous
+            eventSender.keyDown('f'); // flag
+            assertTrue(flaggedTestsTextbox.innerText == '');
         }
     });
 

diff --git a/LayoutTests/fast/harness/results-expected.txt b/LayoutTests/fast/harness/results-expected.txt
index 9abcb25..adbe85a 100644 (file)
--- a/LayoutTests/fast/harness/results-expected.txt
+++ b/LayoutTests/fast/harness/results-expected.txt
@@ -180,16 +180,42 @@ TEST-37: PASS
 TEST-38: PASS
 TEST-38: PASS
 TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
+TEST-39: PASS
+TEST-39: PASS
+TEST-39: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS

diff --git a/LayoutTests/fast/harness/results.html b/LayoutTests/fast/harness/results.html
index eac376f..6a6bf9a 100644 (file)
--- a/LayoutTests/fast/harness/results.html
+++ b/LayoutTests/fast/harness/results.html
@@ -84,10 +84,14 @@ th {
     cursor: default;
 }
 
-.expand-button.current {
+.current {
     color: red;
 }
 
+.current .expand-button {
+    border-color: red;
+}
+
 .expand-button-text {
     position: absolute;
     top: -0.3em;
@@ -95,7 +99,7 @@ th {
 }
 
 .test-link.flagged:after {
-    content: ' (flagged)';
+    content: ' \2691';
 }
 
 .stopped-running-early-message {
@@ -908,11 +912,16 @@ document.addEventListener('mouseout', PixelZoomer.handleMouseOut, false);
 
 var TestNavigator = {};
 
-TestNavigator.currentTestIndex = 0;
-TestNavigator.flaggedTests = {};
+TestNavigator.reset = function() {
+    TestNavigator.currentTestIndex = -1;
+    TestNavigator.flaggedTests = {};
+}
 
 TestNavigator.handleKeyEvent = function(event)
 {
+    if (event.metaKey || event.shiftKey || event.ctrlKey)
+        return;
+
     switch (String.fromCharCode(event.charCode)) {
         case 'i':
             TestNavigator._scrollToFirstTest();
@@ -956,13 +965,17 @@ TestNavigator._scrollToLastTest = function()
 
 TestNavigator._scrollToNextTest = function()
 {
-    if (TestNavigator._setCurrentTest(TestNavigator.currentTestIndex + 1))
+    if (TestNavigator.currentTestIndex == -1)
+        TestNavigator._scrollToFirstTest();
+    else if (TestNavigator._setCurrentTest(TestNavigator.currentTestIndex + 1))
         TestNavigator._scrollToCurrentTest();
 }
 
 TestNavigator._scrollToPreviousTest = function()
 {
-    if (TestNavigator._setCurrentTest(TestNavigator.currentTestIndex - 1))
+    if (TestNavigator.currentTestIndex == -1)
+        TestNavigator._scrollToLastTest();
+    else if (TestNavigator._setCurrentTest(TestNavigator.currentTestIndex - 1))
         TestNavigator._scrollToCurrentTest();
 }
 
@@ -999,21 +1012,27 @@ TestNavigator._toggleCurrentTestFlagged = function()
         testLink.classList.add('flagged');
         TestNavigator.flaggedTests[testName] = 1;
     }
-    
+
+    TestNavigator.updateFlaggedTests();
+}
+
+TestNavigator.updateFlaggedTests = function()
+{
     var flaggedTestTextbox = document.getElementById('flagged-tests');
     if (!flaggedTestTextbox) {
         var flaggedTestContainer = document.createElement('div');
-        flaggedTestContainer.className = 'flagged-test-container';
-        flaggedTestContainer.innerHTML = '<h2>Flagged Tests</h2><div id="flagged-tests" contentEditable></div>';
+        flaggedTestContainer.id = 'flagged-test-container';
+        flaggedTestContainer.innerHTML = '<h2>Flagged Tests</h2><pre id="flagged-tests" contentEditable></pre>';
         document.body.appendChild(flaggedTestContainer);
 
         flaggedTestTextbox = document.getElementById('flagged-tests');
     }
 
-
     var flaggedTests = Object.keys(this.flaggedTests);
     flaggedTests.sort();
-    flaggedTestTextbox.innerHTML = flaggedTests.join('<br>');
+    var separator = document.getElementById('use-newlines').checked ? '\n' : ' ';
+    flaggedTestTextbox.innerHTML = flaggedTests.join(separator);
+    document.getElementById('flagged-test-container').style.display = flaggedTests.length ? '' : 'none';
 }
 
 TestNavigator._setCurrentTest = function(testIndex)
@@ -1024,12 +1043,12 @@ TestNavigator._setCurrentTest = function(testIndex)
 
     var currExpandLink = links[TestNavigator.currentTestIndex];
     if (currExpandLink)
-        currExpandLink.parentNode.classList.remove('current');
+        parentOfType(currExpandLink, 'tr').classList.remove('current');
 
     TestNavigator.currentTestIndex = testIndex;
 
     currExpandLink = links[TestNavigator.currentTestIndex];
-    currExpandLink.parentNode.classList.add('current');
+    parentOfType(currExpandLink, 'tr').classList.add('current');
 
     return true;
 }
@@ -1082,8 +1101,6 @@ function updateExpectedFailures()
         '.expected { display: none; }' : '';
 
     TestNavigator.onlyShowUnexpectedFailuresChanged();
-    // if (onlyShowUnexpectedFailures() && parentOfType(TestNavigator.currentTestLink(), 'tbody').classList.contains('expected'))
-    //     TestNavigator.currentTestLink().classList.remove('current');
 }
 
 var OptionWriter = {};
@@ -1123,6 +1140,12 @@ function updateAllOptions()
     forEach(document.querySelectorAll('#options-menu input'), function(input) { input.onchange(); });
 }
 
+function handleToggleUseNewlines()
+{
+    OptionWriter.save();
+    TestNavigator.updateFlaggedTests();
+}
+
 function handleToggleImagesChange()
 {
     OptionWriter.save();
@@ -1233,6 +1256,7 @@ function generatePage()
         '<div id=options-menu class=hidden-menu>' +
             '<label><input id="unexpected-results" type=checkbox checked onchange="handleUnexpectedResultsChange()">Only unexpected results</label>' +
             '<label><input id="toggle-images" type=checkbox checked onchange="handleToggleImagesChange()">Toggle images</label>' +
+            '<label title="Use newlines instead of spaces to separate flagged tests"><input id="use-newlines" type=checkbox checked onchange="handleToggleUseNewlines()">Use newlines</input>' +
         '</div></div>';
 
     if (globalState().results.interrupted)
@@ -1279,6 +1303,7 @@ function generatePage()
         }
     }
 
+    TestNavigator.reset();
     OptionWriter.apply();
 }
 </script>

diff --git a/LayoutTests/platform/chromium-win/fast/harness/results-expected.txt b/LayoutTests/platform/chromium-win/fast/harness/results-expected.txt
index 9abcb25..adbe85a 100644 (file)
--- a/LayoutTests/platform/chromium-win/fast/harness/results-expected.txt
+++ b/LayoutTests/platform/chromium-win/fast/harness/results-expected.txt
@@ -180,16 +180,42 @@ TEST-37: PASS
 TEST-38: PASS
 TEST-38: PASS
 TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
-TEST-38: PASS
+TEST-39: PASS
+TEST-39: PASS
+TEST-39: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS
+TEST-40: PASS