#51786 closed enhancement (fixed)
Accessibility issue with the logo on the login page
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Login and Registration | Keywords: | has-patch has-testing-info commit |
Focuses: | accessibility | Cc: |
Description
Currently, the login page (wp-login.php) contains a logo, that is created using the following HTML markup:
<h1><a href="https://wordpress.org/">Powered by WordPress</a></h1>
Both the href URL and the link text can be filtered. Through CSS, the link text is placed off-screen, and a logo is added to the link as a background.
The <h1> is required for proper a11y, but it should contain the title for the page below. Something like "Log in" would probably be ideal. Instead, it currently contains a link that takes the user off the page, with no description of the login page's content.
This is causing serious issues for client who are required to conform to WCAG standards, as required by European law.
I'd like to suggested removing the <h1> from around the logo link, and adding an actual <h1> title to the white box below, with relevant textual content.
Attachments (2)
Change History (32)
#2
@
4 years ago
To clarify, the issue can not be resolved using the relevant filter hooks, because the overall HTML structure is hardcoded.
<h1><a href="[filterable URL]">[filterable link text]</a></h1>
Whatever you put into the two filterable elements, it will always be an <h1> with (nothing but) a <a> in it. The two elements need to be separated to make the page properly accessible.
- For a11y, the <h1> should contain (just) the page's title
- The <a> can link to wp.org (or to any other place through filters), but must not be an <h1>
This will require a bit of tinkering with the page's markup and styling, and will introduce a new translation string. And perhaps a new filter for the title text.
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#5
@
4 years ago
Hi, @roytanck! Can you provide a citation for why there's a problem with having a link be the content of the site's H1 heading?
In general, I'd be happy if the logo/link used the site's logo & linked to the site home page if the site's title and logo are set; but I'm not aware of any arguments that it isn't acceptable to have a link inside an H1 element. The H1 should illustrate the primary subject of the page; but having a link within that H1 is not an issue that I'm aware of.
As a note, the primary subject of this page is already fairly clear, particularly given that the entire content of the page consists of a couple of form fields and a couple of links. The topic is made clear in numerous ways, and the H1 is not particularly crucial to understand the purpose of the document.
That said, I think that the clarity could be improved if the primary branding of the screen was associated with the site, rather than with WordPress, and that WordPress was secondary branding.
#6
@
4 years ago
Hi @joedolson. Thank you for looking into this. The problem isn't with the HTML markup as such. It's perfectly fine to have an <a> inside an <h1>. The issue is with the purpose of the <h1>.
https://fae.disability.illinois.edu/rulesets/HEADING_1/
The <h1> should contain an indication of the contents of the current page. A link to another location (whether it's wp.org or the site's home page) is not a description or title for what's found on the current page.
To people using assistive technologies, the purpose of the page is less immediately obvious. They often rely on headings to navigate the page, and don't immediately "see" the form below it.
It's great that WordPress offers filters for the link's href and text, but they can only be used to modify the link. The link text should always describe the link's target (a different page), so in no case will it describe the current page's contents.
This issue came up during an accessibility audit of a client's site (a large government organization in NL). The a11y expert and I concluded that there was no way to fix it using the hooks that WP currently offers. The <h1> and the <a> serve different purposes, and the best option is to split them into two entities. A credit link for WP, and a proper page title.
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#8
@
4 years ago
- Milestone changed from Awaiting Review to Future Release
The ticket was discussed today during the accessibility team's weekly bug-scrub.
The team agrees that there are some aspects of the login page to improve, so the ticket will be considered for one of the next releases.
The main topic argument against changing the h1 is that it might impact backward compatibility, so some more exploration should be done.
#9
@
4 years ago
Backward compatibility is not really an obstacle to this change, but it could present some challenges with multiple existing filters and custom CSS.
I've used a custom stylesheet and some of the filters.
Supporting custom CSS (that's already implemented) is not as difficult I had thought with the replaceable background image and text clipping. Anyone who has replaced the logo with a logo of different dimensions probably edited the .login h1 a
image and/or size without modifying the h1
. It would be good to add a class to the H1 tag to facilitate any overrides, though, for plugins that continue to support older WordPress versions.
Some other considerations:
- The wp-login.php file is also used for registration and password-changing forms.
- The Powered by WordPress message could be moved to the bottom, with site branding at the top.
#10
follow-up:
↓ 11
@
4 years ago
@roytanck Until there's a better option, I recommend using these two filters and replacing the logo image with CSS.
function sab_login_logo_url() {
return home_url( '/' );
}
add_filter( 'login_headerurl', 'sab_login_logo_url' );
function sab_login_logo_text() {
return get_bloginfo( 'name' );
}
add_filter( 'login_headertext', 'sab_login_logo_text' );
The Site Title would fit as link text for a link to the home page, and it would work well enough as a heading.
#11
in reply to:
↑ 10
@
4 years ago
Replying to sabernhardt:
Until there's a better option, I recommend using these two filters and replacing the logo image with CSS.
...
The Site Title would fit as link text for a link to the home page, and it would work well enough as a heading.
Thank you. This is almost exactly what we're currently using.
As for backwards compatibility, I'd assume that at least some plugins/tweaks that style the login screen will have the h1 as part of CSS selectors. So for some 3rd party code, styling could break. Adding more specific CSS classes seems like a good idea.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
8 months ago
#13
@
8 months ago
- Milestone changed from Future Release to 6.7
- Owner set to joedolson
- Status changed from new to accepted
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
7 months ago
This ticket was mentioned in Slack in #accessibility by pamprn. View the logs.
6 months ago
This ticket was mentioned in PR #7386 on WordPress/wordpress-develop by @pamprn.
6 months ago
#16
- Keywords has-patch added
- I've added a "Login" text inside the form and moved the
<h1>
tag to it. So now the WordPress logo is inside a div. - I also added a main tag to the page.
Trac ticket: https://core.trac.wordpress.org/ticket/51786
This ticket was mentioned in Slack in #accessibility by pamprn. View the logs.
6 months ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
6 months ago
This ticket was mentioned in PR #7454 on WordPress/wordpress-develop by @joedolson.
6 months ago
#19
Alternate PR based on https://github.com/WordPress/wordpress-develop/pull/7386
Trac ticket: https://core.trac.wordpress.org/ticket/51786
#20
@
6 months ago
Thanks for the patch @pamprn!
I've created a new version adapted from yours. The new PR contains the following differences:
1) Removed role="main"
. WordPress no longer adds these fallback alternatives, as they are almost never needed anymore.
2) Change the div
that replaced the logo h1
with role="presentation"
on the H1. On reviewing, I note that there are well over 300 plugins that reference the specific pattern #login h1
. That makes changing this element too risky to be a good idea. However, we can change it to a presentational element so that the semantics are removed. I left the CSS changes pointing to the .logo
class, however, as that allows us to write a dev note to encourage people to update their CSS and JS references so that we can remove this in the future.
3) Add h1
headings in the other views: lost password, reset password, and register. The confirm admin email password already had one.
4) Added .screen-reader-text
to the headings. While a visible heading is nice, practically speaking these forms are pretty clear cut. They are single-purpose pages, with an appropriate title
element, and clearly labeled input fields. The visible heading, to me, creates more labor in updating screenshots than any benefit from a visible heading.
#21
follow-up:
↓ 23
@
6 months ago
- Keywords needs-testing has-testing-info added
Instructions for testing:
- Visually inspect the login, registration, lost password, and reset password screens to verify no visual changes.
- Inspect the source code to verify that these pages have two
h1
elements, one on the WP logo withrole="presentation"
and one in theheader
element with descriptive text. - Verify the presence of a
header
element andmain
element in the source code.
This ticket was mentioned in Slack in #core-test by joedolson. View the logs.
6 months ago
#23
in reply to:
↑ 21
@
6 months ago
Test Report
This report validates that the indicated patch addresses the issue.
Environment
- OS: macOS Sonoma 14.7
- WordPress: 6.6.2 ( via Playground )
- Browser: Vivaldi 6.9.3447.48
- Theme: Twenty Twenty-Four
- Active Plugins:none
Actual Results
Visually inspect the login, registration, lost password, and reset password screens to verify no visual changes.
- ✅ Login, Registration, and Lost Password pages show no visual changes.
Inspect the source code to verify that these pages have two h1 elements, one on the WP logo with role="presentation" and one in the header element with descriptive text.
- ✅ Found the two h1 elements as described in the testing instructions. Descriptive texts -
Login
,Lost Password
andRegistration form
found on the respective pages header h1 tags.
Verify the presence of a header element and main element in the source code.
- ✅ Found as expected
Please check attached screenshot for a sample of how it appears in the source code.
#25
@
6 months ago
I think I would rather remove the header
and main
elements from this patch. Landmarks are a best practice, but the login pages on most sites essentially contain only one region right now. I'm more concerned about the possibility of adding semantic elements when authors have already created them (for example, within the output of login_header
or login_footer
hooks). At worst, leaving them out of the patch is a lack of improvement to navigating the page at this time.
Then the code could omit the hidden heading for the email confirmation screen without leaving an empty header
.
<?php if ( 'confirm_admin_email' !== $action ) : ?> <h1 class="screen-reader-text"><?php echo $title; ?></h1> <?php endif; ?>
If the login page ever adds the admin toolbar (#48831), that would make regions and/or a skip link more helpful/necessary.
#26
@
6 months ago
@sabernhardt I'm inclined to agree. It's not the primary purpose of this ticket, and poses some risks.
I did a quick check of plugins that use the login_header
and login_footer
actions, and while they aren't adding landmarks, they *are* inserting HTML elements; and the first I looked at inserted an open div in the header and a close div in the footer. With this patch, that would be broken code. Any extender who is using these hooks to add an additional wrapper will have that issue.
Additionally, if we do *not* wrap the footer in a footer
element, and it ever contains content, we've left orphaned content.
The value of landmarks is in being able to find the sections of the page; the default WordPress version of these forms only have one section, so it's essentially a moot point structurally. If an extender needs additional sections, it probably makes more sense for them to define that.
#27
@
6 months ago
- Keywords commit added
With these changes, I think this is good to commit. It has no visual changes, and the only markup changes are in screen-reader-text.
I did consider whether we should move the h1
above the login_header
action, but decided it was fine where it is. Even if somebody inserts content using the login_header
, the h1
continues to define the primary purpose of the page and describes the content following.
And that would be a misuse of the action, since the login_message
filter is better suited to that.
@joedolson commented on PR #7386:
6 months ago
#29
In r59138
@joedolson commented on PR #7454:
6 months ago
#30
In r59138
Quick mockup of a login page containing a proper title.