WordPress.org

Make WordPress Core

Opened 23 months ago

Last modified 6 months ago

#37305 new defect (bug)

Custom Logo: output invalid according to the W3C

Reported by: henry.wright Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.5
Component: Themes Keywords: has-patch
Focuses: Cc:

Description

The W3C Validator throws an error when the_custom_logo() is used without specifying an itemtype:

The itemprop attribute was specified, but the element is not a property of any item.

Should itemprop="url" be removed from get_custom_logo()?

Attachments (3)

37305.diff (732 bytes) - added by henry.wright 22 months ago.
37305.1.patch (644 bytes) - added by tfrommen 6 months ago.
37305.2.patch (900 bytes) - added by tfrommen 6 months ago.
Include yet another occurrence

Download all attachments as: .zip

Change History (16)

#1 @swissspidy
23 months ago

  • Component changed from General to Themes
  • Summary changed from the_custom_logo() output invalid according to the W3C to Custom Logo: output invalid according to the W3C

#2 @henry.wright
23 months ago

If removing the property isn't an option, perhaps we could recommend theme authors use something like itemscope itemtype="http://schema.org/WebPage" on the <body> element.

#3 @henry.wright
23 months ago

Alternatively, and I think this is the most appropriate solution, we could wrap the logo markup:

<div itemscope itemtype="http://schema.org/Brand">
    <!-- Logo markup here -->
</div>

#4 @karmatosed
23 months ago

I'm not super keen we force or ask theme authors to put anything extra in their body element. If this is needing to be fixed, lets do this in the function once over every theme.

#5 @henry.wright
23 months ago

@karmatosed in that case the fix would be wrap the property with an itemtype or we could simply remove the microdata completely. Which would be preferred?

#6 @henry.wright
22 months ago

The incoming patch wraps the property with the necessary microdata markup. Custom logo markup should now pass the W3C Validator.

@henry.wright
22 months ago

#7 @henry.wright
22 months ago

  • Keywords has-patch added

#8 @ocean90
22 months ago

  • Version set to 4.5

#9 @markcallen
21 months ago

I think making assumptions about the output is inappropriate. There are circumstances where

  • itemptype "Brand" is inappropriate. It may be marking up for "Organization" for example.
  • It is inappropriate to wrap it in anything. The rest of the schema for a type may be included already - this is afterall, just one part of the "Brand" type.

It would be more appropriate to remove the itemprop attributes from the link and img tag and provide a facility for developers to add them (filters and/or function args).

In any event, the get_custom_logo filter that is in place at present provides the ability to wrap the output of the function in the "Brand" (or anything else) markup if that is appropriate for a theme developer.

#10 @henry.wright
21 months ago

@markcallen removing the microdata completely would indeed solve the invalid markup error.

@tfrommen
6 months ago

#11 follow-up: @tfrommen
6 months ago

What needs to be done to get this fixed for core, @ocean90 and @swissspidy?

I just uploaded a new patch, refreshed for current trunk, and with just the removal of the itemprop attribute, as suggested and what makes the most sense to me.

Quite interesting that none of the SEO-affine people find this troublesome...

#12 in reply to: ↑ 11 ; follow-up: @zodiac1978
6 months ago

Replying to tfrommen:

I just uploaded a new patch, refreshed for current trunk, and with just the removal of the itemprop attribute, as suggested and what makes the most sense to me.

Hi Thorsten, there is another itemprop for the logo here: https://github.com/WordPress/WordPress/blob/aaf99e691391cfceb004d848450dbbf3344b1bee/wp-includes/general-template.php#L897

Replying to markcallen:

It would be more appropriate to remove the itemprop attributes from the link and img tag and provide a facility for developers to add them (filters and/or function args).

I think this would be the best way. If you are a theme developer and you know that you have added itemscope itemtype="http://schema.org/WebPage" to the html element for example, you can use the filter or the parameter to add this feature. But the default should be to not add this attribute.

@tfrommen
6 months ago

Include yet another occurrence

#13 in reply to: ↑ 12 @tfrommen
6 months ago

Replying to zodiac1978:

Replying to tfrommen:

I just uploaded a new patch, refreshed for current trunk, and with just the removal of the itemprop attribute, as suggested and what makes the most sense to me.

Hi Thorsten, there is another itemprop for the logo here: https://github.com/WordPress/WordPress/blob/aaf99e691391cfceb004d848450dbbf3344b1bee/wp-includes/general-template.php#L897

Ah, good catch! I just took the original patch as example, and didn't see this somewhat hidden second definition. :)

Updated patch uploaded.

Note: See TracTickets for help on using tickets.