addresses review feedback

addresses review feedback Corrects 876bfa2's removal of these decorative IMGs without replacing their semantic content. thread: #6041 (comment) squash/fixup: 876bfa2 addresses review feedback Adds explicit title-case ARIA-LABEL attributes to elements whose strings are in all caps to support idiomatic translation (cf. #5821, #6003). thread: #6041 (comment) addresses review feedback Refactors MAIN.index-row rather than wrapping the whole DIV.grid in MAIN. thread: #6041 (comment) squash/fixup: 80547cd removes stray ARIA-LABELS (per review feedback) thread: #6041 (comment) addresses review feedback LABEL[for="codename"] was marked HIDDEN to prevent redundant narration in screen readers. Refactoring the LABEL to an H2 that ARIA-labels the containing section streamlines the narration and makes for a more-logical outline. thread: #6041 (comment) squash/fixup: ae56f49 refactors SECTION#codename-hint as DETAILS (per review feedback) ::{before,after} pseudo-elements must be used to style the expand/collapse links ("Show"/"Hide", respectively) because under DIR="ltr" the ::marker pseudo-element will be displayed to the left of the SUMMARY. thread: #6041 (comment) squash/fixup: 1f69640 adds "button" role to non-navigating links (per review feedback) thread: #6041 (comment) undoes autoformatting squash/fixup: 1f69640 fixes tests for refactored DETAILS#codename-hint squash/fixup: c8baf3d fixes unclosed ARIA-LABEL attribute (per review feedback) thread: #6041 (comment) squash/fixup: d6116a2 refactors TIME as H3 of reply ARTICLE (per review feedback) Each reply now consists of an ARTICLE automatically labeled by its H3 TIME. These implicit ARIA semantics are more fluent than those marked explicitly in 1f69640. thread: #6041 (comment) squash/fixup: 1f69640 refactors OUTPUTs as MARKs or Ps (per review feedback) Some "browser / screen reader pairings may not announce or make available the accessible name of the output, even though its a labelable element"[1], apparently including Orca (i.e., on Tails). These elements give the next-best semantics without implying a live region. [1]: https://www.scottohara.me/blog/2019/07/10/the-output-element.html thread: #6041 (review) squash/fixup: 1f69640 squash/fixup: f37fc17 squash/fixup: ae56f49
freedomofpress · Aug 16, 2021 · 6dd4a5c · 6dd4a5c
1 parent 4e19bef
commit 6dd4a5c
Show file tree

Hide file tree

Showing 13 changed files with 93 additions and 116 deletions.
diff --git a/securedrop/sass/source.sass b/securedrop/sass/source.sass
@@ -34,44 +34,41 @@
   text-align: center
   width: 80%
 
-@media only screen and (max-width: 880px)
+@media only screen
   #codename-hint
-    width: 100%
+    summary
+      cursor: pointer
 
-#codename-hint-visible
-  .visible-codename
-    margin: auto
-    padding: auto
-    height: auto
-    overflow: visible
+      &:after
+        content: "Show"
+        display: block
+        float: right
 
-  .hidden-codename
-    margin: 0
-    padding: 0
-    height: 0
-    overflow: hidden
+        text-decoration: none
+        color: $color_securedrop_blue
+        border-bottom: 1px solid rgba(0, 117, 247, 0.4)
 
-  &:target
+      &:hover, &:active
+        &:after
+          color: $color_grimace_blue
+          border-bottom: 1px solid rgba(42, 49, 157, 1)
 
-    .visible-codename
-      margin: 0
-      padding: 0
-      height: 0
-      overflow: hidden
-      border: 0
+    &[open]
+      summary
+        &:after
+          content: "Hide"
 
-    .hidden-codename
-      margin: auto
-      padding: auto
-      height: auto
-      overflow: visible
+@media only screen and (max-width: 880px)
+  #codename-hint
+    width: 100%
 
 input.codename-box
   width: 100%
   padding: 10px
   font-weight: bold
 
 .codename
+  background-color: transparent
   font-family: monospace
   letter-spacing: 0.15em
   font-weight: normal
@@ -160,6 +157,7 @@ p#codename
       font-size: 10px
       font-weight: bold
       padding: 5px
+      margin: 0
 
     a.delete
       float: right
@@ -217,7 +215,7 @@ p#codename
         margin: 0 2%
         padding: 5px 15px
 
-  output#no-replies
+  #no-replies
     display: block
     font-weight: bold
     text-align: center

diff --git a/securedrop/source_app/forms.py b/securedrop/source_app/forms.py
@@ -11,31 +11,21 @@
 
 class LoginForm(FlaskForm):
 
-    codename = PasswordField(
-        "codename",
-        validators=[
-            InputRequired(message=gettext("This field is required.")),
-            Length(
-                1,
-                PassphraseGenerator.MAX_PASSPHRASE_LENGTH,
-                message=gettext(
-                    "Field must be between 1 and "
-                    "{max_codename_len} characters long.".format(
-                        max_codename_len=PassphraseGenerator.MAX_PASSPHRASE_LENGTH
-                    )
-                ),
-            ),
-            # Make sure to allow dashes since some words in the wordlist have them
-            Regexp(r"[\sA-Za-z0-9-]+$", message=gettext("Invalid input.")),
-        ],
-    )
+    codename = PasswordField('codename', validators=[
+        InputRequired(message=gettext('This field is required.')),
+        Length(1, PassphraseGenerator.MAX_PASSPHRASE_LENGTH,
+               message=gettext(
+                   'Field must be between 1 and '
+                   '{max_codename_len} characters long.'.format(
+                       max_codename_len=PassphraseGenerator.MAX_PASSPHRASE_LENGTH))),
+        # Make sure to allow dashes since some words in the wordlist have them
+        Regexp(r'[\sA-Za-z0-9-]+$', message=gettext('Invalid input.'))
+    ])
 
 
 class SubmissionForm(FlaskForm):
-    msg = TextAreaField("msg",
-                        render_kw={"aria-label": gettext("Write a message."),
-                                   "placeholder": gettext("Write a message."),
-                                   })
+    msg = TextAreaField("msg", render_kw={"placeholder": gettext("Write a message."),
+                                          "aria-label": gettext("Write a message.")})
     fh = FileField("fh", render_kw={"aria-label": gettext("Select a file to upload.")})
 
     def validate_msg(self, field: wtforms.Field) -> None:

diff --git a/securedrop/source_templates/base.html b/securedrop/source_templates/base.html
@@ -37,7 +37,7 @@ <h1>{{ gettext("We're sorry, our SecureDrop is currently offline.") }}</h1>
 
           {% if 'logged_in' in session %}
             <nav>
-              <a href="{{ url_for('main.logout') }}" class="btn pull-right" id="logout">{{ gettext('LOG OUT') }}</a>
+              <a href="{{ url_for('main.logout') }}" class="btn pull-right" id="logout" aria-label="{{ gettext('Log Out') }}">{{ gettext('LOG OUT') }}</a>
             </nav>
           {% endif %}
 

diff --git a/securedrop/source_templates/first_submission_flashed_message.html b/securedrop/source_templates/first_submission_flashed_message.html
@@ -1,7 +1,7 @@
 <section class="message">
   <h2>{{ gettext('Success!') }}</h2>
   <p>{{ gettext('Thank you for sending this information to us. Please check back later for replies.') }}
-    <a href="#codename-hint-visible">
+    <a href="#codename-hint">
     {{ gettext('Forgot your codename?') }}
     </a>
   </p>

diff --git a/securedrop/source_templates/flashed.html b/securedrop/source_templates/flashed.html
@@ -4,7 +4,7 @@
   <ul>
   {% for category, message in messages %}
     {% if category != 'banner-warning' %}
-      <li class="flash {{ category }}">
+      <li class="flash {{ category }}" aria-label="{{ gettext(category) }}">
         {{ message }}
       </li>
     {% endif %}

diff --git a/securedrop/source_templates/generate.html b/securedrop/source_templates/generate.html
@@ -7,9 +7,9 @@ <h1>{{ gettext('Welcome') }}</h1>
 
 <p id="codename-explanation" class="explanation">{{ gettext('This codename is what you will use in future visits to receive messages from our team in response to what you submit on the next screen.') }}</p>
 
-<section class="code">
-  <label id="codename-label" for="codename" hidden>{{ gettext('Codename') }}</label>
-  <output id="codename" class="codename" aria-labelledby="codename-label" aria-describedby="codename-instructions codename-explanation">{{ codename }}</output>
+<section class="code" aria-labelledby="codename-heading">
+  <h2 id="codename-heading" class="visually-hidden">{{ gettext('Codename') }}</h2>
+  <mark id="codename" class="codename" aria-describedby="codename-instructions codename-explanation">{{ codename }}</mark>
   <div class="pull-right">
     <form id="regenerate-form" method="post">
       <input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
@@ -28,7 +28,7 @@ <h1>{{ gettext('Welcome') }}</h1>
 <form id="create-form" method="post" action="/create" autocomplete="off">
   <input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
   <input name="tab_id" type="hidden" value="{{ tab_id }}">
-  <button type="submit" class="btn--space pull-right" id="continue-button">
+  <button type="submit" class="btn--space pull-right" id="continue-button" aria-label="{{ gettext('Submit Documents') }}">
      {{ gettext('SUBMIT DOCUMENTS') }}
   </button>
 </form>

diff --git a/securedrop/source_templates/index.html b/securedrop/source_templates/index.html
@@ -39,7 +39,6 @@ <h2 id="security-level-heading" hidden>{{ gettext('How to change your security l
     {% include 'banner_warning_flashed.html' %}
 
     <div class="content">
-    <main>
       <div class="grid">
         {% include 'flashed.html' %}
         {# The div `index-wrap` MUST be ordered this way so that the flexbox works on large and small screens
@@ -52,16 +51,16 @@ <h1><img src="{{ g.logo }}" alt="{{ gettext('{} logo'.format(g.organization_name
             </div>
           </header>
 
-          <div class="index-row">
+          <main class="index-row">
             <section class="index-column index-left" aria-labelledby="first-submission-heading">
-              <div class="index-column-top-container" aria-label="">
+              <div class="index-column-top-container">
               <h2 id="first-submission-heading" class="welcome-text">
                 {{ gettext('First submission') }}
               </h2>
               <p class="extended-welcome-text">{{ gettext('First time submitting to our SecureDrop? Start here.') }}</p>
               </div>
-              <div class="index-column-bottom-container" aria-label="">
-                <a href="{{ url_for('main.generate') }}" id="submit-documents-button" class="btn alt">
+              <div class="index-column-bottom-container">
+                <a href="{{ url_for('main.generate') }}" id="submit-documents-button" class="btn alt" aria-label="{{ gettext('Get Started') }}">
                   {{ gettext('GET STARTED') }}
                 </a>
               </div>
@@ -75,16 +74,15 @@ <h2 id="return-visit-heading" class="welcome-text">
               <p class="extended-welcome-text">{{ gettext('Already have a codename? Check for replies or submit something new.') }}</p>
               </div>
               <div class="index-column-bottom-container">
-                <a href="{{ url_for('main.login') }}" id="login-button" class="btn secondary">
+                <a href="{{ url_for('main.login') }}" id="login-button" class="btn secondary" aria-label="{{ gettext('Log In') }}">
                   {{ gettext('LOG IN') }}
                 </a>
               </div>
             </section>
-          </div>
+          </main>
         </div>
       {% include 'footer.html' %}
       </div>
-    </main>
     </div>
 
 

diff --git a/securedrop/source_templates/login.html b/securedrop/source_templates/login.html
@@ -24,10 +24,10 @@ <h1>{{ gettext('Enter Codename') }}</h1>
   </div>
 
 <div class="pull-right section-spacing">
-  <button type="submit" id="login">
+  <button type="submit" id="login" aria-label="{{ gettext('Continue') }}">
     {{ gettext('CONTINUE') }}
   </button>
-  <a href="{{ url_for('main.index') }}" class="btn secondary" id="cancel">
+  <a href="{{ url_for('main.index') }}" class="btn secondary" id="cancel" aria-label="{{ gettext('Cancel') }}">
     {{ gettext('CANCEL') }}</a>
 </div>
 

diff --git a/securedrop/source_templates/logout.html b/securedrop/source_templates/logout.html
@@ -1,7 +1,7 @@
 {% extends "base.html" %}
 {% block body %}
   <nav>
-    <a href="{{ url_for('main.login') }}" class="btn pull-right"> {{ gettext('LOG IN') }}</a>
+    <a href="{{ url_for('main.login') }}" class="btn pull-right" aria-label="{{ gettext('Log In') }}"> {{ gettext('LOG IN') }}</a>
   </nav>
   <section>
   <h1>{{ gettext('One more thing...') }}</h1>

diff --git a/securedrop/source_templates/lookup.html b/securedrop/source_templates/lookup.html
@@ -2,21 +2,10 @@
 
 {% block body %}
 {% if new_user %}
-<section class="code-reminder pull-left" id="codename-hint" aria-label="{{ gettext('Codename') }}">
-  <div id="codename-hint-visible">
-    <label id="codename-label" for="codename" aria-hidden="true">{{ gettext('Remember, your codename is:') }}</label>
-    {# ARIA-HIDDEN violates axe rule aria-hidden-focus, because we (a) want to
-    hide the superfluous "show"/"hide" links from screen-readers, (b) do not
-    want to remove these elements from sequential navigation with TABINDEX="-1",
-    and (c) do not have recourse in the Source Interface to scripting the
-    ARIA-HIDDEN value dynamically.  Cf. #6031. #}
-    <a id="codename-hint-show" class="show pull-right visible-codename" href="#codename-hint-visible" aria-hidden="true" tabindex="-1">{{ gettext('Show') }}</a>
-    <div id="codename-hint-content" class="hidden-codename codename">
-      <a id="codename-hint-hide" class="pull-right" href="#codename-hint" aria-hidden="true">{{ gettext('Hide') }}</a>
-      <output id="codename" aria-labelledby="codename-label">{{ codename }}</output>
-    </div>
-  </div>
-</section>
+<details class="code-reminder pull-left" id="codename-hint">
+    <summary>{{ gettext('Remember, your codename is:') }}</summary>
+    <mark class="codename">{{ codename }}</mark>
+</details>
 {% endif %}
 
 <div class="center">
@@ -54,13 +43,13 @@ <h2 id="submit-heading" class="headline">{{ gettext('Submit Messages') }}</h2>
   </div>
 
   <div class="pull-right section-spacing">
-    <button type="submit" id="submit-doc-button">
+    <button type="submit" id="submit-doc-button" aria-label="{{ gettext('Submit') }}">
       {{ gettext('SUBMIT') }}
     </button>
 
 
 
-    <a href="{{ url_for('main.lookup') }}" class="btn secondary" id="cancel">
+    <a href="{{ url_for('main.lookup') }}" class="btn secondary" id="cancel" role="button" aria-label="{{ gettext('Cancel') }}">
       {{ gettext('CANCEL') }}
     </a>
   </div>
@@ -72,40 +61,40 @@ <h2 id="replies-heading" class="headline">{{ gettext('Read Replies') }}</h2>
 
 <div id="replies">
   {% if replies %}
-    <output role="status">
+    <p>
       {{ gettext("You have received a reply. To protect your identity in the unlikely event someone learns your codename, please delete all replies when you're done with them. This also lets us know that you are aware of our reply. You can respond by submitting new files and messages above.") }}
-    </output>
+    </p>
     {% for reply in replies %}
-      <article class="reply" aria-labelledby="timestamp-{{ reply.filename }}">
-        <time id="timestamp-{{ reply.filename }}" class="date" title="{{ reply.date|rel_datetime_format }}" datetime="{{ reply.date }}" aria-hidden="true">{{ reply.date|rel_datetime_format(relative=True) }}</time>
+      <article class="reply">
+        <h3 class="date"><time title="{{ reply.date|rel_datetime_format }}" datetime="{{ reply.date }}">{{ reply.date|rel_datetime_format(relative=True) }}</time></h3>
         <form id="delete" class="message" method="post" action="{{ url_for('main.delete') }}" autocomplete="off">
           <input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
           <input type="hidden" name="reply_filename" value="{{ reply.filename }}" autocomplete="off">
-          <a href="#confirm-delete-{{ reply.filename }}" id="delete-reply-{{ reply.filename }}" class="delete" aria-label="{{ gettext('Delete this reply') }}">
+          <a href="#confirm-delete-{{ reply.filename }}" id="delete-reply-{{ reply.filename }}" class="delete" role="button" aria-label="{{ gettext('Delete this reply') }}">
             <span>{{ gettext('Delete') }}</span>
           </a>
           <dialog open id="confirm-delete-{{ reply.filename }}" class="confirm-prompt" aria-labelledby="delete-heading-{{ reply.filename }}">
             <h3 id="delete-heading-{{ reply.filename }}" hidden>Delete reply from {{ reply.date|rel_datetime_format(relative=True) }}?</h3>
             <p>{{ gettext('Delete this reply?') }}
-              <a href="#delete">{{ gettext('Cancel') }}</a>
-              <button type="submit" class="danger" id="confirm-delete-reply-button-{{ reply.filename }}">{{ gettext('DELETE') }}</button>
+              <a href="#delete" role="button">{{ gettext('Cancel') }}</a>
+              <button type="submit" class="danger" id="confirm-delete-reply-button-{{ reply.filename }}" role="button" aria-label="{{ gettext('Delete') }}">{{ gettext('DELETE') }}</button>
             </p>
           </dialog>
         </form>
         <blockquote>{{ reply.decrypted | nl2br }}</blockquote>
       </article>
     {% endfor %}
     <form id="delete-all" method="post" action="{{ url_for('main.batch_delete') }}">
-      <a class="btn danger" href="#delete-all-confirm">{{ gettext('DELETE ALL REPLIES') }}</a>
+      <a class="btn danger" href="#delete-all-confirm" role="button" aria-label="{{ gettext('Delete All Replies') }}">{{ gettext('DELETE ALL REPLIES') }}</a>
       <input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
       <dialog open id="delete-all-confirm" class="hidden-prompt" aria-labelledby="delete-all-heading">
         <h3 id="delete-all-heading">{{ gettext('Are you finished with the replies?') }}</h3>
         <button class="danger" type="submit">{{ gettext('YES, DELETE ALL REPLIES') }}</button>
-        <a class="btn" href="#delete-all">{{ gettext('NO, NOT YET') }}</a>
+        <a class="btn" role="button" href="#delete-all">{{ gettext('NO, NOT YET') }}</a>
       </dialog>
     </form>
   {% else %}
-    <output role="status" id="no-replies" class="explanation">{{ gettext('No Messages') }}</output>
+    <p id="no-replies" class="explanation">{{ gettext('No Messages') }}</p>
   {% endif %}
 </div>
 </section>

diff --git a/securedrop/tests/functional/source_navigation_steps.py b/securedrop/tests/functional/source_navigation_steps.py
@@ -58,25 +58,27 @@ def _source_chooses_to_submit_documents(self):
         self.source_name = codename.text
 
     def _source_shows_codename(self, verify_source_name=True):
-        content = self.driver.find_element_by_id("codename-hint-content")
-        assert not content.is_displayed()
+        # The DETAILS element will be missing the OPEN attribute if it is
+        # closed, hiding its contents.
+        content = self.driver.find_element_by_css_selector("details#codename-hint")
+        assert content.get_attribute("open") is None
 
-        self.safe_click_by_id("codename-hint-show")
+        self.safe_click_by_id("codename-hint")
 
-        self.wait_for(lambda: content.is_displayed())
-        assert content.is_displayed()
-        content_content = self.driver.find_element_by_css_selector("#codename-hint-content output")
+        assert content.get_attribute("open") is not None
+        content_content = self.driver.find_element_by_css_selector("details#codename-hint mark")
         if verify_source_name:
             assert content_content.text == self.source_name
 
     def _source_hides_codename(self):
-        content = self.driver.find_element_by_id("codename-hint-content")
-        assert content.is_displayed()
+        # The DETAILS element will have the OPEN attribute if it is open,
+        # displaying its contents.
+        content = self.driver.find_element_by_css_selector("details#codename-hint")
+        assert content.get_attribute("open") is not None
 
-        self.safe_click_by_id("codename-hint-hide")
+        self.safe_click_by_id("codename-hint")
 
-        self.wait_for(lambda: not content.is_displayed())
-        assert not content.is_displayed()
+        assert content.get_attribute("open") is None
 
     def _source_sees_no_codename(self):
         codename = self.driver.find_elements_by_css_selector(".code-reminder")