Add title blocks.

[skyvine]

Status

Ready for review

Description of Changes

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 #6313.

Changes proposed in this pull request:

Testing

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

I navigated through the pages in both the source and journalist interface and made sure the titles were visible on the tab. I tested this in plain firefox, I'm not overly concerned about browser compatibility because the client is just receiving different static text due to this change, but I can go back and look at it in tor if needed.

Deployment

I have a couple of concerns, I'm not sure if this technically qualifies as "deployment" but this seems like the best place to bring them up.

The heading that I used for the page title in journalist_templates/admin_add_user.html is not wrapped in a gettext call, while most of the others are (other exceptions are categorically different and will be explained shortly). I'm not super familiar with gettext, but IIUC this means that this text will not be translated. This doesn't seem ideal, but I don't think I'm the right person/this is the right issue within which to address that. Technically, the error page is also not in a gettext call, but it's referencing a variable value and I'm assuming that error messages are handled elsewhere, possibly by the browser itself since error codes are well known.

The other exceptions are all because the page did not already contain an appropriate heading, so I added a page title that seemed reasonable. These pages are journalist_templates/col.html, source_templates/index.html, source_templates/login.html, and source_templates/logout.html (see previous comments for the reason why logout has new text). Additionally, source_templates/tor2web-warning.html has the word "WARNING" prepended to the gettext call (again, see previous comments). I could dig through the po files to see if there is a key that seems reasonable, but I'm skeptical there will be. Assuming we want to use the new titles, is there anything I need to do to put these titles on the translations team's radar?

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:

N/A

If you made changes to the system configuration:

N/A

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

N/A

If you made non-trivial code changes:

Discussions about tests for accessibility are taking place in #6314.

Choose one of the following:

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

If you added or updated a production code dependency:

N/A

[cfm]

Nice; thank you, @saffronsnail! I've requested some changes in this review, I believe addressing along the way all of the points you've raised here. Let me know what you think!

[cfm]

Thanks for your changes, @saffronsnail. Per https://app.circleci.com/pipelines/github/freedomofpress/securedrop/548/workflows/6f414f84-bbc5-4b0b-bd9f-4fa4cb4bf654/jobs/67448/tests, it looks like 8ffd659 breaks test_i18n(), because the now-required tab_title will be unset in:

securedrop/securedrop/tests/test_i18n.py

Line 231 in c2f11ef

base = render_template("base.html")

On Monday I'll think about how we can fix this in the existing test_i18n.py test suite—unless you beat me to it. :-)

Finally, as a heads-up: I think this branch is a good candidate for squashing before merge. You can do that before I review again, or I'm happy to do it for you; just let me know what you prefer.

[skyvine]

Commits are squashed! I also added an additional change that fixes the test, not sure if the mechanism is acceptable but it seemed like the simplest thing to do. Instead of rendering base.html it renders error.html, and passes in error={} so that jinja doesn't throw an undefined error. I think this is fine because we're just checking that when a page is rendered in an rtl language the directionality is correct, so it shouldn't matter which template we render.

If this is a problem, my first thought was to inject a tab_title block into the page somehow. If I could get the contents of the template this would be pretty straightforward by using render_template_string and just tacking a block onto the end of it, but when I looked at the flask/jinja code, it's all going straight to rendering, I don't see a convenient helper for getting the contents without simultaneously processing it. I could grab the contents directly, but if I go through flask infrastructure then I end up re-implementing their backend and keeping it in sync sounds annoying. I could grab the contents manually (once I figure out where the files are actually located on the test machine, which seems pretty doable), which is probably what I will do if we need to render exactly base.html for some reason.

[skyvine]

OOC, have you had a chance to look at #6745? It's a WIP but I want to make sure we're in sync about the direction of the remaining work to make sure I'm spending my time efficiently. =)

[cfm]

@saffronsnail in #6738 (comment):

Commits are squashed! I also added an additional change that fixes the test, not sure if the mechanism is acceptable but it seemed like the simplest thing to do. Instead of rendering base.html it renders error.html, and passes in error={} so that jinja doesn't throw an undefined error. I think this is fine because we're just checking that when a page is rendered in an rtl language the directionality is correct, so it shouldn't matter which template we render.

This is great! I was going to look into something similar before recommending it. Using the error.html template seems fine to me, since (as you say) the content of the template is irrelevant for this test.

Two final requests—

1. I've suggested documenting this change in test_i18n.py.
2. Can you edit your commit (and pull request) to remove the now-obsolete references to missing gettext calls?

—and then I'll be ready to merge this right away this week. :-)

[skyvine]

Docstring added to commit, message updated with accurate gettext information!

[cfm]

Looks great, @saffronsnail. Thanks for all your patience with this back-and-forth, and congratulations on your first contribution to SecureDrop!

[skyvine]

Thank you for all the help and feedback! I am happy and proud to help improve this important project.