search

justintadlock / breadcrumb-trail

Official home of the Breadcrumb Trail plugin for WordPress.
GNU General Public License v2.0
136 stars 61 forks source link

Validation errors and warning #22

Open azirer opened 8 years ago

azirer commented 8 years ago

Hi, @justintadlock as I have mentioned here https://github.com/justintadlock/breadcrumb-trail/pull/21 and following your instructions I'm opening this issue. There are 2 validation errors and 1 warning when trying to validate through validator.nu.

The errors are caused by the <meta> tags inside the <ul> in lines 175-176 of inc/breadcrumbs.php and the warning from line 214 of the same file where role="navigation" is not required. after doing those three changes, the page completely validates both at validator.nu and https://developers.google.com/structured-data/testing-tool/ . I can't see that those changes affect anything else, if so please enlighten me.

Regards

samikeijonen commented 8 years ago

It's true that role="navigation" is not required for modern browsers. It's mostly for older IE browsers so I'd keep it around for a while.

For tags inside ul I really don't know what they represent so I don't have opinion about theme :)

justintadlock commented 8 years ago

These are two separate and unrelated issues, so there should be two separate tickets.

role="navigation": I'm in agreement with @samikeijonen on this.

Meta tags: If you have a better idea than simply deleting them, I'm open to hearing those ideas.

samikeijonen commented 8 years ago

Can those meta tags just be outside UL element?

m-e-h commented 8 years ago

@azirer What were the errors stating about the meta? The schema.org webpage for ItemList uses meta tags in nearly the exact same way(third example down). Since BreadcrumbList is a kind of ItemList, I would assume it could use them too.

The opening paragraph for BreadcrumbList mentions itemListOrder but I'm not sure if that means it's assumed, as in it doesn't need declared or is that just how it's most often used?

The convention is that a breadcrumb list has an itemListOrder of ItemListOrderAscending (lower values listed first), and that the first items in this list correspond to the "top" or beginning of the breadcrumb trail

azirer commented 8 years ago

@justintadlock About the role="navigation" ok it is just a warning and for those who want to remove it they can always filter the html output if they want.

@m-e-h The error is Element meta not allowed as child of element ul in this context.

Maybe turn <ul> and all <li> into <div> That would be the same as the third example of https://schema.org/ItemList . I haven't tried to validate it though yet.

However from the moment that the code without the <meta> tags validates at w3c and google structured data and has the desired result meaning it works, it's fine by me until we find another way to include the meta tags again.

Anyway it's up to you guys