WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 14 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 2 years ago.
37305.1.patch (644 bytes) - added by tfrommen 14 months ago.
37305.2.patch (900 bytes) - added by tfrommen 14 months ago.
Include yet another occurrence

Download all attachments as: .zip

Change History (16)

#1 @swissspidy
3 years 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
3 years 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
3 years 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
3 years 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
3 years 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
2 years ago

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

@henry.wright
2 years ago

#7 @henry.wright
2 years ago

  • Keywords has-patch added

#8 @ocean90
2 years ago

  • Version set to 4.5

#9 @markcallen
2 years 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
2 years ago

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

@tfrommen
14 months ago

#11 follow-up: @tfrommen
14 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
14 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
14 months ago

Include yet another occurrence

#13 in reply to: ↑ 12 @tfrommen
14 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.