Opened 9 years ago
Closed 6 years ago
#37305 closed defect (bug) (fixed)
Custom Logo: output invalid according to the W3C
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 5.2 | Priority: | normal |
| Severity: | normal | Version: | 4.5 |
| Component: | Themes | Keywords: | has-patch commit |
| 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 (4)
Change History (21)
#1
@
9 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
#3
@
9 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
@
9 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
@
9 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
@
8 years ago
The incoming patch wraps the property with the necessary microdata markup. Custom logo markup should now pass the W3C Validator.
#9
@
8 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
@
8 years ago
@markcallen removing the microdata completely would indeed solve the invalid markup error.
#11
follow-up:
↓ 12
@
7 years 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:
↓ 13
@
7 years ago
Replying to tfrommen:
I just uploaded a new patch, refreshed for current trunk, and with just the removal of the
itempropattribute, 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.
#13
in reply to:
↑ 12
@
7 years ago
Replying to zodiac1978:
Replying to tfrommen:
I just uploaded a new patch, refreshed for current trunk, and with just the removal of the
itempropattribute, as suggested and what makes the most sense to me.
Hi Thorsten, there is another
itempropfor 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.
#14
@
6 years ago
- Milestone changed from Awaiting Review to 5.2
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
#15
@
6 years ago
Asked to a few SEO experts and the argumentations from @markcallen and @zodiac1978 seem the most appropriate. Core should not make assumption on the output. Also, core is far from supporting schema so this seems more plugins / themes territory and should be removed. Noting also that today, JSON-LD is preferred.
+1 for removal.
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.