WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 13 days ago

#47066 new defect (bug)

Twenty Nineteen: The main element must not appear as a descendant of the section element.

Reported by: albertomake Owned by:
Milestone: 5.3 Priority: normal
Severity: normal Version: 5.1
Component: Bundled Theme Keywords: has-patch needs-dev-note commit
Focuses: accessibility, template Cc:

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)

47066.diff (4.0 KB) - added by ryelle 8 weeks ago.

Download all attachments as: .zip

Change History (8)

#1 @afercia
2 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)

@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.

@ryelle
8 weeks ago

#2 @ryelle
8 weeks 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.

source: HTML Living Standard

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 sections with divs. I don't see any styles relying on the element section, just replacing it will work– and 47066.diff does that :)

#3 @allancole
7 weeks ago

Thanks for sorting this out @ryelle—the patch looks good to me :-)

#4 @desrosj
7 weeks 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.

Last edited 7 weeks ago by desrosj (previous) (diff)

#5 @ianbelanger
6 weeks ago

  • Milestone changed from 5.2.1 to 5.3

Changing milestone as Bundled Themes are only updated during major releases.

#6 @afercia
5 weeks 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 @ianbelanger
13 days ago

  • Keywords needs-dev-note commit added; 2nd-opinion removed

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.

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.

Note: See TracTickets for help on using tickets.