Twenty Nineteen: Duplicate <h1> tags

[williampatton]

In twenty nineteen theme there are situations where duplicate <h1> tags appear on the page. It is best to have just a single <h1> on page for various reasons.

Some details of what was discovered in twenty twenty: ​https://github.com/WordPress/twentytwenty/issues/490 and ​https://github.com/WordPress/twentytwenty/pull/496

The same issue affects twentyninteen. This ticket should be updated with more relevant details.

[adamsilverstein]

@williampatton Do you have any more details/steps to reproduce? I was unable to reproduce this in twentynineteen, are you sure this is an issue?

I tried to reproduce by setting the homepage to a page, when I visited the page title was wrapped in an h1 while the site title was wrapped in <p class="site-title">

Checking the code I see we check:

is_front_page() && is_home() before outputting an h1 - which should only be true when the homepage shows the latest posts in my testing. maybe I am missing some setup step?

And in the docs I found that:

is_home If a static page is set for the front page of the site, this function will return true only on the page you set as the "Posts page". I verified this is correct, even when i set both the posts and home page to the same page, I couldn't get both conditions to be true.

Can you please add exact details about how do you create a condition where you get double h1s and verify this in fact happens in twentynineteen?

[sabernhardt]

I could not find double h1 tags either. However, my special page of posts was missing the h1 heading.

Home page (latest posts): h1 tag on site link, h2 for each post.
Posts page: no h1 tag on site link, no h1 for content, h2 for each post.
Home page (static): no heading around site link, one h1 for page heading ("Home").
Single post/page: no heading tag on site link, one h1 for page heading.
Category archive: no heading tag on site link, one h1 for page heading, h2 for each post.

[adamsilverstein]

Thanks for testing and the detailed notes @sabernhardt!

Not having an h1 isn't great, however probably much less concerning than multiple h1s on the page (because h1 is considered the header for the entire document).

I still don't understand why this wasn't working in twentytwenty and required ​https://github.com/WordPress/twentytwenty/pull/496

[adamsilverstein]

Aha! Figured it out. I was misreading the code, in twentynineteen we had && instead of || in the PR.. as in is_front_page() && is_home().

What really want is to use the h1 when either of these are true (the home page or the blog front page), eg is_front_page() || is_home() except when the home page is a static page (! is_page()) - the exact fix in ​https://github.com/WordPress/twentytwenty/commit/6306c74f6ecf5992811538c915c64e0585b22354

I added this in 48126.diff​

@sabernhardt / @williampatton can you please give that patch a test and see if you get a single h1 on your homepage?

[williampatton]

The patch applies for me and I do not get any duplicate h1 tags with any of the test scenarious I can think of.

Thanks for taking care of this @adamsilverstein :)

[sabernhardt]

The Posts page now has an h1 on the site title; otherwise, my test pages are the same after the patch as what I previously reported. So that would still be zero doubles found, without considering child themes.

One child theme possibility: The Posts page template (home.php) uses single_post_title() to add the page name as a heading above all the posts, which would then be the second h1. Simplifying the conditional to if ( is_front_page() && ! is_page() ) could help in that case.

[adamsilverstein]

@sabernhardt does simplifying the logic that way still work for all use cases? Do we need the is_home check for the case where the posts list is the home page?

[sabernhardt]

@adamsilverstein Well, no, my suggestion of simply removing is_home() does not work in all cases. If I use the child theme scenario I mentioned earlier, then switch back to the default home page of posts without editing/removing the home.php template, I end up with an empty h1 on the home page plus the site-title h1. (Even though this is an edge case, it does show there's a way to break it.)

I can test for another way to fix child theme scenarios, but we should also consider whether the original *needs* fixing. Twenty Fifteen and Twenty Sixteen use the same if ( is_front_page() && is_home() ) logic in their header.php files. That seems to give the site-title a heading tag only in the intended case of a blog-style home page, and I have not seen any different in Twenty Nineteen either. It's also described as the "right" way to target the default homepage template in this Stack Exchange answer:
​https://wordpress.stackexchange.com/questions/224592/if-is-home-is-front-page#239838

Could someone describe an extra h1 instance with (the unpatched) Twenty Nineteen so we can test whether a patch fixes that scenario first? Understanding the problem would be worth more than my hypothetical situations :).

[adamsilverstein]

After reviewing the changes here more carefully, I agree with @sabernhardt - I don't think we need to change anything here.

The reason twentytwenty needed the change is its comparison used is_front_page() || is_home(). In twentynineteen and previous themes used is_front_page() && is_home(). With the logic this way, duplicate h1s do not occur as far as i can tell.

Closing this as worksforme. @williampatton if I have missed something or you feel there is still an issue that needs fixing here, please feel free to reopen this ticket.

[williampatton]

Closing suits me fine, so long as it works is the main thing. Thanks for looking at it and validating things worked well in TwentyNinteen :)