From 408e306ffc52a463fa258e135386a8ad86cffd5f Mon Sep 17 00:00:00 2001
From: Sean Lip <sll@google.com>
Date: Thu, 28 Jul 2016 17:47:43 -0700
Subject: [PATCH] Update aria labels. In the process, refactor lists to comply
 with HTML5 spec to avoid ChromeVox getting confused about how many elements
 are in a list.

---
 accessible/app.component.js            |  2 +-
 accessible/field.component.js          | 47 +++++++++++---------------
 accessible/messages.js                 |  1 +
 accessible/toolbox-tree.component.js   | 46 +++++++++++--------------
 accessible/tree.service.js             | 13 +++----
 accessible/utils.service.js            |  9 ++---
 accessible/workspace-tree.component.js | 30 +++++++++-------
 media/accessible.css                   |  5 ++-
 8 files changed, 74 insertions(+), 79 deletions(-)

diff --git a/accessible/app.component.js b/accessible/app.component.js
index 9de1cc60a..b78b5977b 100644
--- a/accessible/app.component.js
+++ b/accessible/app.component.js
@@ -44,7 +44,7 @@ blocklyApp.AppView = ng.core
     <label aria-hidden="true" hidden id="blockly-argument-input">{{'ARGUMENT_INPUT'|translate}}</label>
     <label aria-hidden="true" hidden id="blockly-argument-menu">{{'ARGUMENT_OPTIONS_LIST'|translate}}</label>
     <label aria-hidden="true" hidden id="blockly-argument-text">{{'TEXT'|translate}}</label>
-    <label aria-hidden="true" hidden id="blockly-block-menu">{{'BLOCK_ACTION_LIST'|translate}}</label>
+    <label aria-hidden="true" hidden id="blockly-block-menu">{{'BLOCK_ACTION_LIST'|translate}} {{'FOR'|translate}}</label>
     <label aria-hidden="true" hidden id="blockly-block-summary">{{'BLOCK_SUMMARY'|translate}}</label>
     <label aria-hidden="true" hidden id="blockly-button">{{'BUTTON'|translate}}</label>
     <label aria-hidden="true" hidden id="blockly-disabled">{{'DISABLED'|translate}}</label>
diff --git a/accessible/field.component.js b/accessible/field.component.js
index 5aa2a74ad..7b1719113 100644
--- a/accessible/field.component.js
+++ b/accessible/field.component.js
@@ -28,44 +28,37 @@ blocklyApp.FieldComponent = ng.core
   .Component({
     selector: 'blockly-field',
     template: `
-    <li [id]="idMap['listItem']" role="treeitem" *ngIf="isTextInput()"
-        [attr.aria-level]="level">
-      <input [id]="idMap['input']" [ngModel]="field.getValue()" (ngModelChange)="field.setValue($event)"
-             [disabled]="disabled" type="text" aria-label="text">
-    </li>
-    <li [id]="idMap['listItem']" role="treeitem" *ngIf="isNumberInput()"
-        [attr.aria-level]="level">
-      <input [id]="idMap['input']" [ngModel]="field.getValue()" (ngModelChange)="field.setValue($event)"
-             [disabled]="disabled" type="number" aria-label="number">
-    </li>
-    <li [id]="idMap['listItem']" role="treeitem" *ngIf="isDropdown()"
-        [attr.aria-labelledBy]="generateAriaLabelledByAttr('blockly-argument-menu', idMap['label'])"
-        [attr.aria-level]="level">
+    <input *ngIf="isTextInput()" [id]="idMap['input']"
+           [ngModel]="field.getValue()" (ngModelChange)="field.setValue($event)"
+           [disabled]="disabled" type="text" aria-label="Press Enter to edit text">
+
+    <input *ngIf="isNumberInput()" [id]="idMap['input']"
+           [ngModel]="field.getValue()" (ngModelChange)="field.setValue($event)"
+           [disabled]="disabled" type="number" aria-label="Press Enter to edit number">
+
+    <div *ngIf="isDropdown()"
+         [attr.aria-labelledBy]="generateAriaLabelledByAttr('blockly-argument-menu', idMap['label'])">
       <label [id]="idMap['label']">{{'CURRENT_ARGUMENT_VALUE'|translate}} {{field.getText()}}</label>
       <ol role="group">
         <li [id]="idMap[optionValue]" role="treeitem" *ngFor="#optionValue of getOptions()"
-            [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap[optionValue + 'Button'], 'blockly-button')"
-            [attr.aria-level]="level + 1">
+            [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap[optionValue + 'Button'], 'blockly-button')">
           <button [id]="idMap[optionValue + 'Button']" (click)="handleDropdownChange(field, optionValue)"
                   [disabled]="disabled">
             {{optionText[optionValue]}}
           </button>
         </li>
       </ol>
-    </li>
-    <li [id]="idMap['listItem']" role="treeitem" *ngIf="isCheckbox()"
-        [attr.aria-level]="level">
+    </div>
+
+    <div *ngIf="isCheckbox()">
       // Checkboxes are not currently supported.
-    </li>
-    <li [id]="idMap['listItem']" role="treeitem" *ngIf="isTextField() && hasVisibleText()"
-        [attr.aria-labelledBy]="utilsService.generateAriaLabelledByAttr('blockly-argument-text', idMap['label'])"
-        [attr.aria-level]="level">
-      <label [id]="idMap['label']">
-        {{field.getText()}}
-      </label>
-    </li>
+    </div>
+
+    <label [id]="idMap['label']" *ngIf="isTextField() && hasVisibleText()">
+      {{field.getText()}}
+    </label>
     `,
-    inputs: ['field', 'level', 'index', 'parentId', 'disabled'],
+    inputs: ['field', 'index', 'parentId', 'disabled'],
     pipes: [blocklyApp.TranslatePipe]
   })
   .Class({
diff --git a/accessible/messages.js b/accessible/messages.js
index e4ca99d5f..022de027b 100644
--- a/accessible/messages.js
+++ b/accessible/messages.js
@@ -58,6 +58,7 @@ Blockly.Msg.COPY_TO_MARKED_SPOT = 'copy to marked spot';
 Blockly.Msg.TOOLBOX = 'Toolbox';
 Blockly.Msg.WORKSPACE = 'Workspace';
 Blockly.Msg.ANY = 'any';
+Blockly.Msg.FOR = 'for';
 Blockly.Msg.STATEMENT = 'statement';
 Blockly.Msg.VALUE = 'value';
 Blockly.Msg.CUT_BLOCK_MSG = 'Cut block: ';
diff --git a/accessible/toolbox-tree.component.js b/accessible/toolbox-tree.component.js
index b0278a3c7..6bd889744 100644
--- a/accessible/toolbox-tree.component.js
+++ b/accessible/toolbox-tree.component.js
@@ -40,57 +40,51 @@ blocklyApp.ToolboxTreeComponent = ng.core
             [attr.aria-level]="level + 1">
           <label #label [id]="idMap['label']">{{'BLOCK_ACTION_LIST'|translate}}</label>
           <ol role="group" *ngIf="displayBlockMenu">
-            <li #workspaceCopy [id]="idMap['workspaceCopy']" role="treeitem"
+            <li [id]="idMap['workspaceCopy']" role="treeitem"
                 [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap['workspaceCopyButton'], 'blockly-button')"
                 [attr.aria-level]="level + 2">
-              <button #workspaceCopyButton [id]="idMap['workspaceCopyButton']"
-                      (click)="copyToWorkspace()">
+              <button [id]="idMap['workspaceCopyButton']" (click)="copyToWorkspace()">
                 {{'COPY_TO_WORKSPACE'|translate}}
               </button>
             </li>
-            <li #blockCopy [id]="idMap['blockCopy']" role="treeitem"
+            <li [id]="idMap['blockCopy']" role="treeitem"
                 [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap['blockCopyButton'], 'blockly-button')"
                 [attr.aria-level]="level + 2">
-              <button #blockCopyButton
-                      [id]="idMap['blockCopyButton']"
-                      (click)="copyToClipboard()">
+              <button [id]="idMap['blockCopyButton']" (click)="copyToClipboard()">
                 {{'COPY_TO_CLIPBOARD'|translate}}
               </button>
             </li>
-            <li #sendToSelected [id]="idMap['sendToSelected']" role="treeitem"
+            <li [id]="idMap['sendToSelected']" role="treeitem"
                 [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap['sendToSelectedButton'], 'blockly-button', !canBeCopiedToMarkedConnection())"
                 [attr.aria-level]="level + 2">
-              <button #sendToSelectedButton
-                      [id]="idMap['sendToSelectedButton']"
-                      (click)="copyToMarkedSpot()"
+              <button [id]="idMap['sendToSelectedButton']" (click)="copyToMarkedSpot()"
                       [disabled]="!canBeCopiedToMarkedConnection()">
                 {{'COPY_TO_MARKED_SPOT'|translate}}
               </button>
             </li>
           </ol>
         </li>
-        <div *ngFor="#inputBlock of block.inputList; #i=index">
-          <blockly-field *ngFor="#field of inputBlock.fieldRow; #j=index"
-                         [field]="field" [level]="level + 1" [disabled]="true">
-          </blockly-field>
+        <template ngFor #inputBlock [ngForOf]="block.inputList" #i="index">
+          <li role="treeitem" [id]="idMap['listItem' + i]" [attr.aria-level]="level + 1">
+            <blockly-field *ngFor="#field of inputBlock.fieldRow" [field]="field" [disabled]="true">
+            </blockly-field>
+          </li>
+
           <blockly-toolbox-tree *ngIf="inputBlock.connection && inputBlock.connection.targetBlock()"
-                                [block]="inputBlock.connection.targetBlock()"
-                                [displayBlockMenu]="false"
-                                [level]="level + 1">
+                                [block]="inputBlock.connection.targetBlock()" [level]="level + 1"
+                                [displayBlockMenu]="false">
           </blockly-toolbox-tree>
-          <li #listItem1 [id]="idMap['listItem' + i]" role="treeitem"
+          <li [id]="idMap['inputList' + i]" role="treeitem"
               *ngIf="inputBlock.connection && !inputBlock.connection.targetBlock()"
-              [attr.aria-labelledBy]="generateAriaLabelledByAttr('blockly-argument-text', idMap['listItem' + i + 'Label'])"
               [attr.aria-level]="level + 1">
-            <!--TODO(madeeha): i18n here will need to happen in a different way due to the way grammar changes based on language.-->
-            <label #label [id]="idMap['listItem' + i + 'Label']">
-              {{utilsService.getInputTypeLabel(inputBlock.connection)}}
-              {{utilsService.getBlockTypeLabel(inputBlock)}} needed:
+            <label>
+              {{utilsService.getInputTypeLabel(inputBlock.connection)}} {{utilsService.getBlockTypeLabel(inputBlock)}} needed:
             </label>
           </li>
-        </div>
+        </template>
       </ol>
     </li>
+
     <blockly-toolbox-tree *ngIf= "block.nextConnection && block.nextConnection.targetBlock()"
                           [level]="level"
                           [block]="block.nextConnection.targetBlock()"
@@ -123,7 +117,7 @@ blocklyApp.ToolboxTreeComponent = ng.core
             'blockCopyButton', 'sendToSelected', 'sendToSelectedButton']);
       }
       for (var i = 0; i < this.block.inputList.length; i++){
-        elementsNeedingIds.push('listItem' + i, 'listItem' + i + 'Label')
+        elementsNeedingIds.push('listItem' + i, 'inputList' + i);
       }
       this.idMap = this.utilsService.generateIds(elementsNeedingIds);
       if (this.isTopLevel) {
diff --git a/accessible/tree.service.js b/accessible/tree.service.js
index f0e6c8710..2108c0a81 100644
--- a/accessible/tree.service.js
+++ b/accessible/tree.service.js
@@ -246,12 +246,13 @@ blocklyApp.TreeService = ng.core
         if (e.keyCode == 13) {
           // Enter key. The user wants to interact with a button or an input
           // field.
-          if (activeDesc.children.length == 1) {
-            var child = activeDesc.children[0];
-            if (child.tagName == 'BUTTON') {
-              child.click();
-            } else if (child.tagName == 'INPUT') {
-              child.focus();
+          var currentChild = activeDesc;
+          while (currentChild.children.length) {
+            currentChild = currentChild.children[0];
+            if (currentChild.tagName == 'BUTTON') {
+              currentChild.click();
+            } else if (currentChild.tagName == 'INPUT') {
+              currentChild.focus();
             }
           }
         } else if (e.keyCode == 9) {
diff --git a/accessible/utils.service.js b/accessible/utils.service.js
index 34a57de21..d2f258731 100644
--- a/accessible/utils.service.js
+++ b/accessible/utils.service.js
@@ -48,13 +48,10 @@ blocklyApp.UtilsService = ng.core
       return attrValue;
     },
     getInputTypeLabel: function(connection) {
-      // Returns an upper case string in the case of official input type names.
-      // Returns the lower case string 'any' if any official input type qualifies.
-      // The differentiation between upper and lower case signifies the difference
-      // between an input type (BOOLEAN, LIST, etc) and the colloquial english term
-      // 'any'.
+      // Returns the input type name, or 'any' if any official input type
+      // qualifies.
       if (connection.check_) {
-        return connection.check_.join(', ').toUpperCase();
+        return connection.check_.join(', ');
       } else {
         return Blockly.Msg.ANY;
       }
diff --git a/accessible/workspace-tree.component.js b/accessible/workspace-tree.component.js
index 879a1906e..f706dbd82 100644
--- a/accessible/workspace-tree.component.js
+++ b/accessible/workspace-tree.component.js
@@ -51,17 +51,24 @@ blocklyApp.WorkspaceTreeComponent = ng.core
           </ol>
         </li>
 
-        <div *ngFor="#inputBlock of block.inputList; #i = index">
-          <blockly-field *ngFor="#field of inputBlock.fieldRow" [field]="field" [level]="level + 1"></blockly-field>
+        <template ngFor #inputBlock [ngForOf]="block.inputList" #i="index">
+          <li role="treeitem" [id]="idMap['listItem' + i]" [attr.aria-level]="level + 1" *ngIf="inputBlock.fieldRow.length">
+            <blockly-field *ngFor="#field of inputBlock.fieldRow" [field]="field">
+            </blockly-field>
+          </li>
+
           <blockly-workspace-tree *ngIf="inputBlock.connection && inputBlock.connection.targetBlock()"
                                   [block]="inputBlock.connection.targetBlock()" [level]="level + 1"
                                   [tree]="tree">
           </blockly-workspace-tree>
-          <li #inputList [attr.aria-level]="level + 1" [id]="idMap['inputList' + i]"
+          <li #inputList [id]="idMap['inputList' + i]" role="treeitem"
+              *ngIf="inputBlock.connection && !inputBlock.connection.targetBlock()"
               [attr.aria-labelledBy]="generateAriaLabelledByAttr('blockly-menu', idMap['inputMenuLabel' + i])"
-              *ngIf="inputBlock.connection && !inputBlock.connection.targetBlock()" (keydown)="treeService.onKeypress($event, tree)">
-            <!-- TODO(madeeha): i18n here will need to happen in a different way due to the way grammar changes based on language. -->
-            <label [id]="idMap['inputMenuLabel' + i]"> {{utilsService.getInputTypeLabel(inputBlock.connection)}} {{utilsService.getBlockTypeLabel(inputBlock)}} needed: </label>
+              [attr.aria-level]="level + 1"
+              (keydown)="treeService.onKeypress($event, tree)">
+            <label [id]="idMap['inputMenuLabel' + i]">
+              {{utilsService.getInputTypeLabel(inputBlock.connection)}} {{utilsService.getBlockTypeLabel(inputBlock)}} needed:
+            </label>
             <ol role="group">
               <li *ngFor="#fieldButtonInfo of fieldButtonsInfo"
                   [id]="idMap[fieldButtonInfo.baseIdKey]" role="treeitem"
@@ -75,7 +82,7 @@ blocklyApp.WorkspaceTreeComponent = ng.core
               </li>
             </ol>
           </li>
-        </div>
+        </template>
       </ol>
     </li>
 
@@ -318,11 +325,10 @@ blocklyApp.WorkspaceTreeComponent = ng.core
       });
       for (var i = 0; i < this.block.inputList.length; i++) {
         var inputBlock = this.block.inputList[i];
-        if (inputBlock.connection && !inputBlock.connection.targetBlock()) {
-          that.idKeys.push(
-              'inputList' + i, 'inputMenuLabel' + i, 'markSpot' + i,
-              'markSpotButton' + i, 'paste' + i, 'pasteButton' + i);
-        }
+        that.idKeys.push(
+            'inputList' + i, 'inputMenuLabel' + i, 'markSpot' + i,
+            'markSpotButton' + i, 'paste' + i, 'pasteButton' + i,
+            'listItem' + i);
       }
     },
     ngDoCheck: function() {
diff --git a/media/accessible.css b/media/accessible.css
index 651c74328..5a68527ca 100644
--- a/media/accessible.css
+++ b/media/accessible.css
@@ -4,6 +4,9 @@
 .blocklyTree .blocklyActiveDescendant > label,
 .blocklyTree .blocklyActiveDescendant > div > label,
 .blocklyActiveDescendant > button,
-.blocklyActiveDescendant > input {
+.blocklyActiveDescendant > input,
+.blocklyActiveDescendant > blockly-field > label,
+.blocklyActiveDescendant > blockly-field > input,
+.blocklyActiveDescendant > blockly-field > div > label {
   outline: 2px dotted #00f;
 }