From 19b5fc6b227b0df14fb0b7011dd209c080fca244 Mon Sep 17 00:00:00 2001
From: Awais Jibran <awaisdar001@gmail.com>
Date: Wed, 13 Jun 2018 11:27:15 +0500
Subject: [PATCH] Screen reader issue with structure of LMS course outline

Potential solutions:
1. Removal of the "role" attributes on the "li" elements and on the "ol"
2. Wrapping of the section/subsection titles in heading tags (<h4>-<h5>) that map to the correct nesting level of the content in the course (this is done correctly for the top-level elements)
EDUCATOR-3035
---
 .../test/acceptance/pages/lms/course_home.py  | 25 ++++++++-----------
 .../course-outline-fragment.html              | 25 ++++++++-----------
 .../tests/views/test_course_outline.py        |  2 --
 3 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/common/test/acceptance/pages/lms/course_home.py b/common/test/acceptance/pages/lms/course_home.py
index 9ff0472483..b1563ec040 100644
--- a/common/test/acceptance/pages/lms/course_home.py
+++ b/common/test/acceptance/pages/lms/course_home.py
@@ -170,6 +170,7 @@ class CourseOutlinePage(PageObject):
 
         # Click the subsection's first problem and ensure that the page finishes
         # reloading
+        units[0].location_once_scrolled_into_view  # pylint: disable=W0104
         units[0].click()
 
         self._wait_for_course_section(section_title, subsection_title)
@@ -304,23 +305,17 @@ class CourseOutlinePage(PageObject):
         '''
         Expands all parts of the collapsible outline.
         '''
-        section_button_selector = '.section-name.accordion-trigger'
-        subsection_button_selector = '.subsection-text.accordion-trigger'
-        self._expand_outline_fold(section_button_selector)
-        self._expand_outline_fold(subsection_button_selector)
+        expand_button_search_results = self.q(
+            css='#expand-collapse-outline-all-button'
+        ).results
 
-    def _expand_outline_fold(self, fold_selector):
-        '''
-        Ensures an outline fold is loaded, then clicks it open.
-        '''
-        folds_as_elements = self.q(css=fold_selector)
-        self.wait_for_element_visibility(
-            fold_selector, "'{}' is visible".format(fold_selector)
-        )
+        if not expand_button_search_results:
+            return
 
-        for fold_element in folds_as_elements:
-            if not self._is_html_element_aria_expanded(fold_element):
-                fold_element.click()
+        expand_button = expand_button_search_results[0]
+
+        if not self._is_html_element_aria_expanded(expand_button):
+            expand_button.click()
 
 
 class CourseSearchResultsPage(CoursePage):
diff --git a/openedx/features/course_experience/templates/course_experience/course-outline-fragment.html b/openedx/features/course_experience/templates/course_experience/course-outline-fragment.html
index 07c900bc74..5f89fd2e9c 100644
--- a/openedx/features/course_experience/templates/course_experience/course-outline-fragment.html
+++ b/openedx/features/course_experience/templates/course_experience/course-outline-fragment.html
@@ -26,13 +26,12 @@ course_sections = blocks.get('children')
         </button>
         <ol class="block-tree accordion"
             id="course-outline-block-tree"
-            role="presentation"
             aria-labelledby="expand-collapse-outline-all-button">
         % for section in course_sections:
-            <% 
-            section_is_auto_opened = section.get('resume_block') is True 
+            <%
+            section_is_auto_opened = section.get('resume_block') is True
             %>
-                <li class="outline-item section" role="heading">
+                <li class="outline-item section">
                     <button class="section-name accordion-trigger"
                             aria-expanded="${ 'true' if section_is_auto_opened else 'false' }"
                             aria-controls="${ section['id'] }_contents"
@@ -45,7 +44,6 @@ course_sections = blocks.get('children')
                     </button>
                     <ol class="outline-item accordion-panel ${ '' if section_is_auto_opened else 'is-hidden' }"
                         id="${ section['id'] }_contents"
-                        role="region"
                         aria-labelledby="${ section['id'] }">
             % for subsection in section.get('children', []):
                 <%
@@ -53,7 +51,7 @@ course_sections = blocks.get('children')
                 completed_prereqs = gated_content[subsection['id']]['completed_prereqs'] if gated_subsection else False
                 subsection_is_auto_opened = subsection.get('resume_block') is True
                 %>
-                            <li class="subsection accordion ${ 'current' if subsection['resume_block'] else '' }" role="heading">
+                      <li class="subsection accordion ${ 'current' if subsection['resume_block'] else '' }">
                 % if gated_subsection and not completed_prereqs:
                                 <a href="${ subsection['lms_web_url'] }">
                                     <button class="subsection-text prerequisite-button"
@@ -61,9 +59,9 @@ course_sections = blocks.get('children')
                                     <span class="menu-icon icon fa fa-lock"
                                             aria-hidden="true">
                                     </span>
-                                    <span class="subsection-title">
+                                    <h4 class="subsection-title">
                                         ${ subsection['display_name'] }
-                                    </span>
+                                    </h4>
                                     <div class="details prerequisite">
                                         ${ _("Prerequisite: ") }
                                             <%
@@ -79,9 +77,9 @@ course_sections = blocks.get('children')
                                             aria-controls="${ subsection['id'] }_contents">
                                         <span class="fa fa-chevron-right ${ 'fa-rotate-90' if subsection_is_auto_opened else '' }"
                                               aria-hidden="true"></span>
-                                        <span class="subsection-title">
+                                        <h4 class="subsection-title">
                                             ${ subsection['display_name'] }
-                                        </span>
+                                        </h4>
                     % if subsection.get('complete'):
                                         <span class="complete-checkmark fa fa-check"></span>
                     % endif
@@ -150,7 +148,6 @@ course_sections = blocks.get('children')
                 % if not gated_subsection or (gated_subsection and completed_prereqs):
                                 <ol class="outline-item accordion-panel ${ '' if subsection_is_auto_opened else 'is-hidden' }"
                                     id="${ subsection['id'] }_contents"
-                                    role="region"
                                     aria-labelledby="${ subsection['id'] }"
                                 >
                     % for vertical in subsection.get('children', []):
@@ -159,9 +156,9 @@ course_sections = blocks.get('children')
                                             href="${ vertical['lms_web_url'] }"
                                             id="${ vertical['id'] }">
                                             <div class="vertical-details">
-                                                <span class="vertical-title">
-                                                    ${ vertical['display_name'] }
-                                                </span>
+                                              <div class="vertical-title">
+                                                ${ vertical['display_name'] }
+                                              </div>
                                             </div>
                         % if vertical.get('complete'):
                                                 <span class="complete-checkmark fa fa-check"></span>
diff --git a/openedx/features/course_experience/tests/views/test_course_outline.py b/openedx/features/course_experience/tests/views/test_course_outline.py
index 406389aee5..b9f61b3630 100644
--- a/openedx/features/course_experience/tests/views/test_course_outline.py
+++ b/openedx/features/course_experience/tests/views/test_course_outline.py
@@ -538,10 +538,8 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT
 
             return "<olclass=\"outline-itemaccordion-panel" + is_hidden_string + "\"" \
                    "id=\"" + url + "_contents\"" \
-                   "role=\"region\"" \
                    "aria-labelledby=\"" + url + "\"" \
                    ">"
-
         # Course tree
         course = self.course
         chapter = course.children[0]