-
Notifications
You must be signed in to change notification settings - Fork 687
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
surface view-level h1
s (or equivalents) in page title
s (WCAG 2.4.2)
#6313
Comments
Adding more context in light of having added good first issue and hackathon labels: The page
securedrop/securedrop/source_templates/base.html Lines 9 to 11 in 6170370
But for us to conform with the Web Content Accessibility Guidelines 2.0 Success Criterion 2.4.2, we need to provide descriptive page titles. So instead of being static, the This would likely involve adding a new jinja title block that can be filled from the templates that use |
Hello. I am new to this project in particular and ad-hoc free software work in general. I think I have a solution for this issue though, using the below python function. I've looked at this on the journalist site, have not yet looked at the source site. I'm also not sure where this function should go since it's common to both the source and the journalist, and I'd rather not copy it into both directories. So far I've been testing with it defined in 95 @pass_context
96 def get_descriptive_title(c):
97 found_opening_h1_tag = False
98 for uut in c.blocks['body']:
99 uut = uut(c)
100 app.logger.error(f'Item: {uut}')
101 for text in uut:
102 if not found_opening_h1_tag:
103 if '<h1' in text:
104 found_opening_h1_tag = True
105 else:
106 return text
107 app.logger.error(f'Current test value: {text}')
108 app.logger.error(f'No h1 block found on page, returning empty string!')
109 return ''
...
117 app.jinja_env.globals.update(get_descriptive_title=get_descriptive_title) |
I think I have a workable diff. I put the function in a new file It looks like several of the pages in the source interface don't actually have One of the files whose text changed is The second file whose text changed is Finally, there are still several files which lack Journalist Templates - Files without h1 block post-diff
Source Templates - Files without h1 block post-diff
Complete Diffdiff --git a/securedrop/journalist_app/__init__.py b/securedrop/journalist_app/__init__.py
index 905baf64e..a09cd4e3e 100644
--- a/securedrop/journalist_app/__init__.py
+++ b/securedrop/journalist_app/__init__.py
@@ -14,6 +14,7 @@ from journalist_app.sessions import Session, session
from journalist_app.utils import get_source
from models import InstanceConfig
from sdconfig import SecureDropConfig
+from utils import create_get_descriptive_title_function
from werkzeug import Response
from werkzeug.exceptions import HTTPException, default_exceptions
@@ -94,6 +95,7 @@ def create_app(config: SecureDropConfig) -> Flask:
app.jinja_env.trim_blocks = True
app.jinja_env.lstrip_blocks = True
app.jinja_env.globals["version"] = version.__version__
+ app.jinja_env.globals["get_descriptive_title"] = create_get_descriptive_title_function(app.logger)
app.jinja_env.filters["rel_datetime_format"] = template_filters.rel_datetime_format
app.jinja_env.filters["filesizeformat"] = template_filters.filesizeformat
app.jinja_env.filters["html_datetime_format"] = template_filters.html_datetime_format
diff --git a/securedrop/journalist_templates/base.html b/securedrop/journalist_templates/base.html
index 32594493a..13d642765 100644
--- a/securedrop/journalist_templates/base.html
+++ b/securedrop/journalist_templates/base.html
@@ -4,7 +4,7 @@
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
- <title>{{ g.organization_name }}</title>
+ <title>{{ get_descriptive_title() }} | {{ g.organization_name }}</title>
<link rel="stylesheet" href="/static/css/journalist.css">
diff --git a/securedrop/source_app/__init__.py b/securedrop/source_app/__init__.py
index a7301d71b..2950b51ba 100644
--- a/securedrop/source_app/__init__.py
+++ b/securedrop/source_app/__init__.py
@@ -17,6 +17,7 @@ from sdconfig import SecureDropConfig
from source_app import api, info, main
from source_app.decorators import ignore_static
from source_app.utils import clear_session_and_redirect_to_logged_out_page
+from utils import create_get_descriptive_title_function
def get_logo_url(app: Flask) -> str:
@@ -70,6 +71,7 @@ def create_app(config: SecureDropConfig) -> Flask:
app.jinja_env.globals["version"] = version.__version__
# Exported to source templates for being included in instructions
app.jinja_env.globals["submission_key_fpr"] = config.JOURNALIST_KEY
+ app.jinja_env.globals["get_descriptive_title"] = create_get_descriptive_title_function(app.logger)
app.jinja_env.filters["rel_datetime_format"] = template_filters.rel_datetime_format
app.jinja_env.filters["nl2br"] = template_filters.nl2br
app.jinja_env.filters["filesizeformat"] = template_filters.filesizeformat
diff --git a/securedrop/source_templates/base.html b/securedrop/source_templates/base.html
index 75e088d61..ac42f1efa 100644
--- a/securedrop/source_templates/base.html
+++ b/securedrop/source_templates/base.html
@@ -6,9 +6,9 @@
<meta name="viewport" content="width=device-width, initial-scale=1">
<meta name="robots" content="noindex,nofollow">
{% if g.organization_name == "SecureDrop" %}
- <title>{{ g.organization_name }} | {{ gettext('Protecting Journalists and Sources') }}</title>
+ <title>{{ get_descriptive_title() }} | {{ g.organization_name }} | {{ gettext('Protecting Journalists and Sources') }}</title>
{% else %}
- <title>{{ g.organization_name }} | {{ gettext('SecureDrop') }}</title>
+ <title>{{ get_descriptive_title() }} | {{ g.organization_name }} | {{ gettext('SecureDrop') }}</title>
{% endif %}
<link rel="stylesheet" href="/static/css/source.css">
diff --git a/securedrop/source_templates/error.html b/securedrop/source_templates/error.html
index 082832161..fa83ed90b 100644
--- a/securedrop/source_templates/error.html
+++ b/securedrop/source_templates/error.html
@@ -1,6 +1,6 @@
{% extends "base.html" %}
{% block body %}
-<h2>{{ gettext('Server error') }}</h2>
+<h1>{{ gettext('Server error') }}</h1>
<p>{{ gettext('Sorry, the website encountered an error and was unable to complete your request.') }}</p>
diff --git a/securedrop/source_templates/flash_message.html b/securedrop/source_templates/flash_message.html
index 81f9dbaea..e8e6c2263 100644
--- a/securedrop/source_templates/flash_message.html
+++ b/securedrop/source_templates/flash_message.html
@@ -1,4 +1,4 @@
{%- if declarative %}
-<h2>{{ declarative }}</h2>
+<h1>{{ declarative }}</h1>
{% endif -%}
<p {% if not declarative %} class="icon"{% endif %}>{{ msg_contents }}</p>
diff --git a/securedrop/source_templates/generate.html b/securedrop/source_templates/generate.html
index 7f2e381fd..af1a8ae7c 100644
--- a/securedrop/source_templates/generate.html
+++ b/securedrop/source_templates/generate.html
@@ -2,7 +2,7 @@
{% import 'utils.html' as utils %}
{% block body %}
-<h2>{{ gettext('Get Your Codename') }}</h2>
+<h1>{{ gettext('Get Your Codename') }}</h1>
<p id="codename-explanation" class="explanation">
{{ gettext('A <em>codename</em> in SecureDrop functions as both your username and your password.') }}
diff --git a/securedrop/source_templates/login.html b/securedrop/source_templates/login.html
index a7308627f..25e22c4c1 100644
--- a/securedrop/source_templates/login.html
+++ b/securedrop/source_templates/login.html
@@ -3,7 +3,7 @@
{% include 'flashed.html' %}
-<h2>{{ gettext('Enter Codename') }}</h2>
+<h1>{{ gettext('Enter Codename') }}</h1>
<form method="post" action="{{ url_for('main.login') }}" autocomplete="off">
<input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
diff --git a/securedrop/source_templates/logout.html b/securedrop/source_templates/logout.html
index 65f608741..16242ad55 100644
--- a/securedrop/source_templates/logout.html
+++ b/securedrop/source_templates/logout.html
@@ -5,7 +5,7 @@
{{ gettext('LOG IN') }}</a>
</nav>
<section>
- <h2>{{ gettext('One more thing...') }}</h2>
+ <h1>{{ gettext('Logout - Additional Action Needed') }}</h1>
<p>
{{ gettext('Click the <img src={icon} alt="" width="16" height="16"> <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')) }}
</p>
diff --git a/securedrop/source_templates/lookup.html b/securedrop/source_templates/lookup.html
index 9389f2be0..733005366 100644
--- a/securedrop/source_templates/lookup.html
+++ b/securedrop/source_templates/lookup.html
@@ -10,10 +10,10 @@
<section aria-labelledby="submit-heading">
{% if allow_document_uploads %}
- <h2 id="submit-heading">{{ gettext('Submit Files or Messages') }}</h2>
+ <h1 id="submit-heading">{{ gettext('Submit Files or Messages') }}</h1>
<p>{{ gettext('You can submit any kind of file, a message, or both.') }}</p>
{% else %}
- <h2 id="submit-heading">{{ gettext('Submit Messages') }}</h2>
+ <h1 id="submit-heading">{{ gettext('Submit Messages') }}</h1>
{% endif %}
<p>
diff --git a/securedrop/source_templates/notfound.html b/securedrop/source_templates/notfound.html
index 9abd23cb4..b57d3fb8d 100644
--- a/securedrop/source_templates/notfound.html
+++ b/securedrop/source_templates/notfound.html
@@ -1,6 +1,6 @@
{% extends "base.html" %}
{% block body %}
-<h2>{{ gettext('Page not found') }}</h2>
+<h1>{{ gettext('Page not found') }}</h1>
<p id="page-not-found">{{ gettext("Sorry, we couldn't locate what you requested.") }}</p>
diff --git a/securedrop/source_templates/tor2web-warning.html b/securedrop/source_templates/tor2web-warning.html
index 16a12a729..46d412fc8 100644
--- a/securedrop/source_templates/tor2web-warning.html
+++ b/securedrop/source_templates/tor2web-warning.html
@@ -1,6 +1,6 @@
{% extends "base.html" %}
{% block body %}
-<h2>{{ gettext('Proxy Service Detected') }}</h2>
+<h1>{{ gettext('Warning! Proxy Service Detected') }}</h1>
<div class="center">
{% include 'flashed.html' %}
</div>
diff --git a/securedrop/source_templates/use-tor-browser.html b/securedrop/source_templates/use-tor-browser.html
index fa82c4851..e9c9479f3 100644
--- a/securedrop/source_templates/use-tor-browser.html
+++ b/securedrop/source_templates/use-tor-browser.html
@@ -1,6 +1,6 @@
{% extends "base.html" %}
{% block body %}
-<h2>{{ gettext('You Should Use Tor Browser') }}</h2>
+<h1>{{ gettext('You Should Use Tor Browser') }}</h1>
<p>{{ gettext('If you are not using Tor Browser, you <strong>may not be anonymous</strong>.') }}</p>
<p>{{ gettext('If you want to submit information to SecureDrop, we <strong>strongly advise you</strong> to install Tor Browser and use it to access our site safely and anonymously.') }}
</p>
diff --git a/securedrop/source_templates/why-public-key.html b/securedrop/source_templates/why-public-key.html
index 31241a170..71fdde0c1 100644
--- a/securedrop/source_templates/why-public-key.html
+++ b/securedrop/source_templates/why-public-key.html
@@ -1,6 +1,6 @@
{% extends "base.html" %}
{% block body %}
-<h2>{{ gettext("Why download the team's public key?") }}</h2>
+<h1>{{ gettext("Why download the team's public key?") }}</h1>
<p>{{ gettext("SecureDrop encrypts files and messages after they are submitted. Encrypting messages and files before submission can provide an extra layer of security before your data reaches the SecureDrop server.") }}
</p>
<p>{{ gettext("If you are already familiar with the GPG encryption software, you may wish to encrypt your submissions yourself. To do so:") }}
diff --git a/securedrop/utils.py b/securedrop/utils.py
new file mode 100644
index 000000000..d9c01f167
--- /dev/null
+++ b/securedrop/utils.py
@@ -0,0 +1,17 @@
+from jinja2.runtime import Context
+from jinja2.utils import pass_context
+from logging import Logger
+
+def create_get_descriptive_title_function(logger: Logger):
+ @pass_context
+ def get_descriptive_title(context: Context):
+ for block_func in context.blocks['body']:
+ block = list(block_func(context))
+ for (possibly_h1_tag, possibly_h1_content) in zip(block, block[1:]):
+ if '<h1' in possibly_h1_tag:
+ return possibly_h1_content
+
+ logger.error(f'No h1 block found on page {context.name}, returning empty string!')
+ return ''
+
+ return get_descriptive_title |
Updated the above comment - originally posted without the diff included, and fixed the formatting so that the code blocks are collapsible. Sorry for the noise! |
The above pull request is marked as WIP. I need to think about how to test the new function and look at documentation. As stated in the PR, I think the requirement for web pages to have a descriptive h1 statement which takes users depending on screen readers into account needs to be documented somewhere. The change is mostly the same as diff from my earlier comment. I tweaked the for loop so that the second item in the tuple is an index instead of directly containing the string which is theoretically better if python is copying values instead of shuffling pointers, not sure but the strategy should be fine either way. Also fixed some linter errors. Finally, |
For what is worth, I fully second this change. (And take note that there is a broader conversation to be had about the balance between casual conversation and clarity, that also comes up from time to time during localization.) In my opinion the considerations on the source likely state of mind are very relevant. 🎯
Same. Smaller change, that also makes sense to me. 👍 |
With my apologies for having put @saffronsnail on the spot in #6725, I've belatedly filled in this stub ticket with (a) the assumptions @eaon and I have made about it and (b) the parameters clarified in #6725 (review). Even with the approach so simplified, what we need before it can be implemented is the blessing of our UX designer and security engineers. So I'm looping in:
|
Hi all, I'm wondering if there's any sort of expected timeline for the review to happen? I understand that you weren't necessarily expecting to look at this issue at this moment and I'm sure there's a lot of other things going on, I'm just wondering if the timeline looks more like days or weeks or months? The new design for the change looks very straightforward, so I should be able to submit something soon after the review is completed. |
On Mon, Jan 30, 2023 at 05:42:02AM -0800, Skyler Ferris wrote:
Hi all, I'm wondering if there's any sort of expected timeline for the
review to happen? I understand that you weren't necessarily expecting
to look at this issue at this moment and I'm sure there's a lot of
other things going on, I'm just wondering if the timeline looks more
like days or weeks or months? The new design for the change looks very
straightforward, so I should be able to submit something soon after
the review is completed.
@saffronsnail, let me chat with the team tomorrow and get back to you.
|
We discussed this today and concluded that the security and UX considerations we'd originally raised in #5972 can be considered resolved. So, please feel free to work on this at your convenience, @saffronsnail, and I should be able to review your pull request with a week's turnaround time. :-) |
Per the Web Content Accessibilty Guidelines 2.0, Success Criterion 2.4.2, all pages should have descriptive titles to help users orient themselves within the website, particularly for users who take advantage of screen readers while browsing the web. This commit adds a new `tab_title` block to the base templates for the source and journalist, as well as the index page of the source (it does not derive from base). It is named this way so that it is clear that this block will NOT add a "title" to the *contents* of the page, eg a heading at the beginning of the content. The other files had descriptive titles added. In most cases, these titles are a copy-paste of a `gettext` call already in the page, but there are a few exceptions. `journalist_templates/admin_add_user.html` currently does not use a `gettext` call for the heading that is appropriate for the title. `journalist_templates/error.html` also does not, although it is getting the name of an error from a variable, so I assume that translation is handled elsewhere in this case. `journalist_templates/col.html`, `source_templates/index.html`, and `source_templates/login.html` did not have headings that were appropriate for page titles, so I invented new titles for these. I am not a translator, so these are not in `gettext` calls. `source_templates/logout.html` and `source_templates/tor2web-warning.html` had headings which could be used for titles, but the text was either modified or replaced to make sure that users who are relying soley on the tab title, without the ability to conveniently skim the contents to better understand the length and nature of the text, receive communication about the importance and urgency of the contents of these pages from the title itself. Fixes freedomofpress#6313
Per the Web Content Accessibilty Guidelines 2.0, Success Criterion 2.4.2, all pages should have descriptive titles to help users orient themselves within the website, particularly for users who take advantage of screen readers while browsing the web. This commit adds a new `tab_title` block to the base templates for the source and journalist, as well as the index page of the source (it does not derive from base). It is named this way so that it is clear that this block will NOT add a "title" to the *contents* of the page, eg a heading at the beginning of the content. The other files had descriptive titles added. In most cases, these titles are a copy-paste of a `gettext` call already in the page, but there are a few exceptions. `journalist_templates/admin_add_user.html` currently does not use a `gettext` call for the heading that is appropriate for the title. `journalist_templates/error.html` also does not, although it is getting the name of an error from a variable, so I assume that translation is handled elsewhere in this case. `journalist_templates/col.html`, `source_templates/index.html`, and `source_templates/login.html` did not have headings that were appropriate for page titles, so I invented new titles for these. I am not a translator, so these are not in `gettext` calls. `source_templates/logout.html` and `source_templates/tor2web-warning.html` had headings which could be used for titles, but the text was either modified or replaced to make sure that users who are relying soley on the tab title, without the ability to conveniently skim the contents to better understand the length and nature of the text, receive communication about the importance and urgency of the contents of these pages from the title itself. Also update the `test_orgname_is_changed` test so that the assertions do not have a dependency on the page title. Fixes freedomofpress#6313
Per the Web Content Accessibilty Guidelines 2.0, Success Criterion 2.4.2, all pages should have descriptive titles to help users orient themselves within the website, particularly for users who take advantage of screen readers while browsing the web. This commit adds a new `tab_title` block to the base templates for the source and journalist, as well as the index page of the source (it does not derive from base). It is named this way so that it is clear that this block will NOT add a "title" to the *contents* of the page, eg a heading at the beginning of the content. The other files had descriptive titles added. In most cases, these titles are a copy-paste of a `gettext` call already in the page, but there are a few exceptions. `journalist_templates/admin_add_user.html` currently does not use a `gettext` call for the heading that is appropriate for the title. `journalist_templates/error.html` also does not, although it is getting the name of an error from a variable, so I assume that translation is handled elsewhere in this case. `journalist_templates/col.html`, `source_templates/index.html`, and `source_templates/login.html` did not have headings that were appropriate for page titles, so I invented new titles for these. I am not a translator, so these are not in `gettext` calls. `source_templates/logout.html` and `source_templates/tor2web-warning.html` had headings which could be used for titles, but the text was either modified or replaced to make sure that users who are relying soley on the tab title, without the ability to conveniently skim the contents to better understand the length and nature of the text, receive communication about the importance and urgency of the contents of these pages from the title itself. Also update 2 tests. `test_orgname_is_changed` previously had a dependency on the exact page title, now it just checks for the presense of the correct text. Additionally, `test_i18n` now renders `error.html` instead of `base.html` so that the `tab_title` block is defined. Fixes freedomofpress#6313
Per the Web Content Accessibilty Guidelines 2.0, Success Criterion 2.4.2, all pages should have descriptive titles to help users orient themselves within the website, particularly for users who take advantage of screen readers while browsing the web. This commit adds a new `tab_title` block to the base templates for the source and journalist, as well as the index page of the source (it does not derive from base). It is named this way so that it is clear that this block will NOT add a "title" to the *contents* of the page, eg a heading at the beginning of the content. The other files had descriptive titles added. In most cases, these titles are a copy-paste of a `gettext` call already in the page. In the exception cases, a `gettext` call has been added. `source_templates/logout.html` and `source_templates/tor2web-warning.html` had headings which could be used for titles, but the text was either modified or replaced to make sure that users who are relying soley on the tab title, without the ability to conveniently skim the contents to better understand the length and nature of the text, receive communication about the importance and urgency of the contents of these pages from the title itself. Also update 2 tests. `test_orgname_is_changed` previously had a dependency on the exact page title, now it just checks for the presense of the correct text. Additionally, `test_i18n` now renders `error.html` instead of `base.html` so that the `tab_title` block is defined. Fixes freedomofpress#6313 Co-authored-by: Cory Francis Myers <[email protected]>
Per #5972:
A few belated notes per #6725 (review):
h1
across views, the "equivalents" here will usually be the per-viewh2
.title
automatically from these headings. As it is, a simple fillable Jinja{% block title %}Your Title Here{% endblock %}
is adequate, even though it requires developers to keep the title in sync manually.We do need sign-off from our security engineers and UX designer, to whom I'll flag this question in a comment below.(resolved per surface view-levelh1
s (or equivalents) in pagetitle
s (WCAG 2.4.2) #6313 (comment))The text was updated successfully, but these errors were encountered: