#47066 closed defect (bug) (fixed)
Twenty Nineteen: The main element must not appear as a descendant of the section element.
Reported by: | albertomake | Owned by: | audrasjb |
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | 5.1 |
Component: | Bundled Theme | Keywords: | has-patch has-dev-note |
Focuses: | accessibility, template | Cc: | |
PR Number: |
Description
In the version 1.3 of the Twenty Nineteen theme there is a markup error in different templates:
- 404.php
- archive.php
- image.php
- index.php
- page.php
- search.php
- single.php
where the main element appears as a descendant of a section:
... <section id="primary" class="content-area"> <main id="main" class="site-main"> ... </main><!-- #main --> </section><!-- #primary --> ...
I would suggest replacing ¨section¨ for ¨div¨.
Attachments (1)
Change History (23)
#1
@
6 months ago
- Component changed from Themes to Bundled Theme
- Focuses accessibility added
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 5.2.1
- Type changed from enhancement to defect (bug)
#2
@
6 months ago
- Keywords has-patch added; needs-patch removed
Here's the relevant part of the spec:
A hierarchically correct main element is one whose ancestor elements are limited to html, body, div, form without an accessible name, and autonomous custom elements.
The applicable part is that main
has a special requirement about all its parent elements, and section
is not allowed. So yes, the fix is to revert https://github.com/WordPress/twentynineteen/pull/190, and replace those section
s with div
s. I don't see any styles relying on the element section
, just replacing it will work– and 47066.diff does that :)
#4
@
6 months ago
- Keywords 2nd-opinion added
I'm concerned about the potential for negative side effects this could have on sites.
Any site that targets section#primary anythingelse {}
or section.content-area anythingelse {}
would see their theme break, and potentially severely because this is a high level element in the DOM.
Any site that uses Twenty Nineteen and has any custom CSS defined either in a child theme that does not override all of these templates, or added through the Customizer could be effected.
This could be a low number of sites since Twenty Nineteen is relatively new, but I don't know that we can safely roll out this change without very advanced notice.
#5
@
6 months ago
- Milestone changed from 5.2.1 to 5.3
Changing milestone as Bundled Themes
are only updated during major
releases.
#6
@
5 months ago
- Summary changed from The main element must not appear as a descendant of the section element. to Twenty Nineteen: The main element must not appear as a descendant of the section element.
#7
@
5 months ago
- Keywords needs-dev-note commit added; 2nd-opinion removed
Any site that targets
section#primary anythingelse {}
orsection.content-area anythingelse {}
would see their theme break, and potentially severely because this is a high level element in the DOM.
While I agree with @desrosj's above statement, I think that is a serious enough issue that it has to be fixed, regardless of the dangers. It is currently milestoned for 5.3
and I believe that we have enough time to warn users and devs of this upcoming change. Going to mark this with needs-dev-note
and commit
BTW 47066.diff still applies cleanly.
This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.
2 months ago
#10
@
2 months ago
Hi,
I drafted the dev note for this ticket: https://docs.google.com/document/d/1TdpT53dSI2_x1EQ1mEwEqcGqulyOHpSZFjBCWKK9szo/edit?usp=sharing
Waiting for commit
action now.
This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.
2 months ago
#12
@
2 months ago
@ryelle @ianbelanger we (accessibility team) discussed this ticket during today's extra bug-scrub focused on the 5.3 milestone. When you have a chance: anything left here? It would be good to commit this change as soon as possible in the release cycle so that a dev note can be published on Make well in advance.
#13
@
2 months ago
It all looks good to me @afercia, as long as the patch still applies cleanly. I can't currently test it myself. @audrasjb's dev note also looks good. I give it the go ahead to commit
#15
@
2 months ago
Thanks @ianbelanger. The dev note is drafted on Make/Core and is ready to publish.
#18
@
2 months ago
Just saw this and wanted to add a few comments to the ticket.
I recognize that this is invalid markup. It's unfortunate that we shipped it this way.
I did some quick searching for Twenty Nineteen child themes on GitHub to see if I could find any instances that would be affected by this. I didn't see any in the first couple of pages of results.
However, I suspect that the most likely situation where a section#primary
or section.content-area
would be used would be in custom CSS in the admin. The reason I think that is that it's not the ideal way to target those elements, so it's likely going to show up in small tweaks done by people who may not be well versed in CSS. Unfortunately there is not going to be an easy way to know the extent of that usage.
The people who do that are also almost never going to read a dev note on the make blog. So we just need to understand that we're likely going to break some people's sites without warning.
I know that since it's technically invalid markup, it could affect assistive technologies. Do we have any documented examples of them not functioning properly because of this or is it just theoretical?
It's probably the right choice, but just think we'd have a bit more acknowledgement and discussion of the risks first.
#19
@
2 months ago
So we just need to understand that we're likely going to break some people's sites without warning.
@earnjam I guess we all share your concerns :) That's the reason why this change needs to be merger early in the release cycle. A dev note will be published on Make today or tomorrow so I wouldn't say it's "without warning".
A positive action everybody in the community can take is to communicate this change to the broadest audience as possible, starting with their personal network.
I know that since it's technically invalid markup, it could affect assistive technologies. Do we have any documented examples of them not functioning properly because of this or is it just theoretical?
I'd say it's not only "technically invalid", it's also a bit embarrassing for an official theme. As per your question: no, I don't have documented examples of how this specific case might affect user agents and assistive technologies. Yes, I do have examples of how other cases of invalid markup affected user agents and assistive technologies. This can't be really predicted, because it also depends on the actual content within the <main>
element.
@albertomake thanks for your report and welcome to Trac. Looks like this was introduced in https://github.com/WordPress/twentynineteen/pull/190.
Suggestion: When in doubt about correct usage of HTML5 semantic elements, a good reading of the specs may help, together with passing the generated markup through the W3C validator at https://validator.w3.org/nu/#textarea /Cc @allancole @kjellr
I'd like to propose to fix this as soon as possible in the next minor release.