Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#6405 Add annotations wcag #7081

Closed
wants to merge 12 commits into from
Closed

#6405 Add annotations wcag #7081

wants to merge 12 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 25, 2023

Status

Ready for review / Work in progress

Description of Changes

Added abbreviation descriptions to the html pages.
Fixes #6405.

Changes proposed in this pull request:

  • add abbr tag to html pages

Testing

How should the reviewer test this PR?
Write out any special testing steps here.

  • when looking through the page ensure the correct words have abbreviations

Deployment

Any special considerations for deployment? Consider both:

  1. Upgrading existing production instances.
  2. New installs.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make test) pass in the development container

If you made changes to securedrop-admin:

  • Linting and tests (make -C admin test) pass in the admin development container

If you made changes to the system configuration:

If you added or removed a file deployed with the application:

  • I have updated AppArmor rules to include the change

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

Choose one of the following:

  • [ x] I have opened a PR in the docs repo for these changes, or will do so later
  • [ x] I would appreciate help with the documentation
  • These changes do not require documentation

If you added or updated a reference to a production code dependency:

Production code dependencies are defined in:

  • admin/requirements.in
  • admin/requirements-ansible.in
  • securedrop/requirements/python3/requirements.in
  • securedrop/requirements/python3/translation.in (used in the build
    container)

If you changed another requirements.in file that applies only to development
or testing environments, then no diff review is required, and you can skip
(remove) this section.

Choose one of the following:

  • I have performed a diff review and pasted the contents to the packaging wiki
  • I would like someone else to do the diff review
  • I am silencing an alert related to a production dependency, because (please explain below):

@ghost ghost self-requested a review as a code owner November 25, 2023 20:32
Copy link
Member

@cfm cfm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amnak1284, thanks for proposing these changes. I've left some feedback on specific annotations. In brief, some of these abbreviations benefit from annotation more than others in context, and we'd like to balance their benefit against their cost as changes requiring new translations.

@@ -4,15 +4,15 @@

{% block body %}
{% if user.is_totp %}
<h1>{{ gettext('Enable FreeOTP') }}</h1>
<h1>{{ gettext('Enable <abbr title="two-factor authentication application">FreeOTP</abbr>') }}</h1>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"FreeOTP" is the name of a specific application, so it doesn't need a generic annotation.

<p>
{{ gettext("You're almost done! To finish resetting your two-factor authentication, follow the instructions below to set up FreeOTP. Once you've added the entry for your account in the app, enter one of the 6-digit codes from the app to confirm that two-factor authentication is set up correctly.") }}
</p>

<ol>
<li>{{ gettext('Install FreeOTP on your phone') }}</li>
<li>{{ gettext('Open the FreeOTP app') }}</li>
<li>{{ gettext('Tap the QR code symbol at the top') }}</li>
<li>{{ gettext('Tap the <abbr title="quick response">QR</abbr> code symbol at the top') }}</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't annotate an initialism that's (a) in common use and (b) cited far more widely (463 million Google results) than its expansion (2.1 million results).

@@ -23,7 +23,7 @@ <h1>{{ gettext('Enable FreeOTP') }}</h1>
<mark id="shared-secret">{{ user.formatted_otp_secret }}</mark>
<p>{{ gettext("Once you have paired FreeOTP with this account, enter the 6-digit verification code below:") }}</p>
{% else %}
<h1>{{ gettext('Enable YubiKey (OATH-HOTP)') }}</h1>
<h1>{{ gettext('Enable YubiKey (<abbr title="HMAC based one-time password algorithm">OATH-HOTP</abbr>)') }}</h1>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<h1>{{ gettext('Enable YubiKey (<abbr title="HMAC based one-time password algorithm">OATH-HOTP</abbr>)') }}</h1>
<h1>{{ gettext('Enable YubiKey (OATH-<abbr title="HMAC-based one-time password">HOTP</abbr>)') }}</h1>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed this change

@@ -85,7 +85,7 @@ <h2 id="users-heading" class="visually-hidden">Users</h2>

<a href="{{ url_for('admin.manage_config') }}" class="btn icon icon-edit section-spacing-inline"
id="update-instance-config" aria-label="{{ gettext('Update instance config') }}">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See next comment:

Suggested change
id="update-instance-config" aria-label="{{ gettext('Update instance config') }}">
id="update-instance-config" aria-label="{{ gettext('Update instance configuration') }}">

@@ -85,7 +85,7 @@ <h2 id="users-heading" class="visually-hidden">Users</h2>

<a href="{{ url_for('admin.manage_config') }}" class="btn icon icon-edit section-spacing-inline"
id="update-instance-config" aria-label="{{ gettext('Update instance config') }}">
{{ gettext('INSTANCE CONFIG') }}
{{ gettext('INSTANCE <abbr title="Configuration">CONFIG</abbr>') }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go ahead and just expand this, as we have in: :-)

<h1>{{ gettext('Instance Configuration') }}</h1>

Suggested change
{{ gettext('INSTANCE <abbr title="Configuration">CONFIG</abbr>') }}
{{ gettext('INSTANCE CONFIGURATION') }}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed this

@@ -94,7 +94,7 @@ <h2 id="config-preventuploads">{{ gettext('Submission Preferences') }}</h2>
<section aria-labelledby="config-testalert">
<h2 id="config-testalert">{{ gettext('Alerts') }}</h2>

<p>{{ gettext('Send an encrypted email to verify if OSSEC alerts work correctly:') }}</p>
<p>{{ gettext('Send an encrypted email to verify if <abbr title="Open Source HIDS SECurity">OSSEC</abbr> alerts work correctly:') }}</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OSSEC is the name of an application, and we only ever refer to it this way. I don't think it needs expanding.

@@ -3,7 +3,7 @@
{{ gettext('Powered by') }} <b>SecureDrop {{ version }}</b>.
</p>
<p id="footer-advisory">
{{ gettext('Please note: Sharing sensitive information may put you at risk, even when using Tor and SecureDrop.') }}
{{ gettext('Please note: Sharing sensitive information may put you at risk, even when using <abbr title="The Onion Router">Tor</abbr> and SecureDrop.') }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is worth expanding. It's no longer the official initialism of the Tor project or network, and users will reach this page via Tor Browser already.

@@ -22,7 +22,7 @@
<div id="browser-security-level" class="warning" role="alert">
<p>{{ gettext('Your Tor Browser\'s <a id="disable-js" href=""><b>Security Level</b></a> is too low. Use the <img src="{icon}" alt="&quot;Security Level&quot;"> button in your browser’s toolbar to change it.').format(icon=url_for("static", filename="i/font-awesome/white/guard.svg")) }}</p>
</div>
{# Warning bubble to help TB users disable JavaScript with Tor Browser's Security Levels
{# Warning bubble to help <abbr title="Tor Browser">TB</abbr> users disable JavaScript with <abbr title="The Onion Router">Tor</abbr> Browser's Security Levels
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a server-side Jinja comment for developers, so users will never see it, and HTML doesn't work here anyway. :-)

@@ -10,7 +10,7 @@
<section>
<h2>{{ gettext('One more thing...') }}</h2>
<p>
{{ gettext('Click the <img src={icon} alt="" width="16" height="16">&nbsp;<b>New Identity</b> button in your Tor Browser\'s toolbar. This will clear your Tor Browser activity data on this device.').format(icon=url_for('static', filename='i/torbroom.png')) }}
{{ gettext('Click the <img src={icon} alt="" width="16" height="16">&nbsp;<b>New Identity</b> button in your <abbr title="The Onion Router">Tor</abbr> Browser\'s toolbar. This will clear your Tor Browser activity data on this device.').format(icon=url_for('static', filename='i/torbroom.png')) }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

@@ -7,7 +7,7 @@ <h2>{{ gettext('Proxy Service Detected') }}</h2>
<div class="center">
{% include 'flashed.html' %}
</div>
<p>{{ gettext('You appear to be using a Tor proxy service to access SecureDrop. Proxy services do not protect your anonymity.') }}
<p>{{ gettext('You appear to be using a <abbr title="The Onion Router">Tor</abbr> proxy service to access SecureDrop. Proxy services do not protect your anonymity.') }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

@cfm cfm self-assigned this Nov 28, 2023
@cfm
Copy link
Member

cfm commented Jan 3, 2024

@amnak1284, I'm going to close this pull request for now, but feel free to reopen it if you're able to make these changes!

@cfm cfm closed this Jan 3, 2024
@amnak613 amnak613 mentioned this pull request Oct 30, 2024
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

annotate abbreviations and SecureDrop jargon (WCAG 3.1.3, 3.1.4)
2 participants