Breadcrumb Rich Snippets Microdata

Using Google rich snippets tools
(http://www.google.com/webmasters/tools/richsnippets) and I found out
that the microdata format is not completely correct.

I think the changes I made fixed that issue.
<a href='#crh-start'></a><a href='#crh-data-%7B%22processed%22%3A%20%5B%22https%3A//github.com/zencart/zc-v1-series/pull/134%23issuecomment-52660891%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16437458%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16437460%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23issuecomment-52863218%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16567162%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16584306%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16596267%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16597828%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16693219%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16696791%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16704290%22%5D%2C%20%22comments%22%3A%20%7B%22General%20Comment%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23issuecomment-52660891%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Sorry%20for%20the%20%5C%22reopened%5C%22.%20Accidentally%20deleted%20my%20branch.%5Cr%5Cn%5Cr%5CnThanks%22%2C%20%22created_at%22%3A%20%222014-08-19T16%3A31%3A26Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1428276%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/IronLady%22%7D%7D%2C%20%7B%22body%22%3A%20%22The%20improvements%20to%20richsnippet%20markup%20for%20breadcrumbs%20look%20good%2C%20and%20appear%20to%20pass%20the%20tests%20on%20Google%27s%20validator.%5Cr%5Cn%5Cr%5Cn%3A%2B1%3A%20%22%2C%20%22created_at%22%3A%20%222014-08-21T00%3A16%3A50Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/404472%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/drbyte%22%7D%7D%5D%2C%20%22title%22%3A%20%22General%20Comment%22%7D%2C%20%22Pull%20d51c4d706406a825b9f1313bfad3331d89e70a0c%20includes/classes/breadcrumb.php%208%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16437458%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22There%20are%20a%20couple%20concatenation%20breaks%20around%20the%20%60span%60%20which%20are%20unnecessary.%22%2C%20%22created_at%22%3A%20%222014-08-19T19%3A28%3A58Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22Please%20remove%20the%20unnecessary%20concatenations%20around%20the%20%60itemprop%60%20span.%22%2C%20%22created_at%22%3A%20%222014-08-21T20%3A37%3A31Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22Thanks%20for%20the%20comment.%20%5Cr%5CnBut%2C%20I%20don%27t%20think%20that%20it%27s%20unnecessary%20%28the%20breaks%29.%20%5Cr%5CnI%20was%20thinking%20that%20I%20need%20to%20put%20one%20more%20break%20inside%20%3Ccode%3Eitemscope%3C/code%3E%20span.%5Cr%5Cn%60%60%60php%5Cr%5Cn%24trail_string%20.%3D%20%27%20%20%3Cspan%20itemscope%20itemtype%3D%5C%22http%3A//data-vocabulary.org/Breadcrumb%5C%22%3E%27%20.%20%27%3Ca%20itemprop%3D%5C%22url%5C%22%20href%3D%5C%22%27%20.%20%24this-%3E_trail%5B%24i%5D%5B%27link%27%5D%20.%20%27%5C%22%3E%27%20.%20%27%3Cspan%20itemprop%3D%5C%22title%5C%22%3E%27%20.%20%24this-%3E_trail%5B%24i%5D%5B%27title%27%5D%20.%20%27%3C/span%3E%27%20.%20%27%3C/a%3E%27%20.%20%27%3C/span%3E%27%3B%5Cr%5Cn%60%60%60%5Cr%5Cn%5Cr%5CnIt%20can%20be%20something%20like%20a%20%5C%22mark%5C%22%20for%20us%20to%20easily%20edit%20it%2C%20IN%20CASE%20there%20are%20any%20schema%20markup%20changes%20in%20the%20future%20%28who%20knows%29.%20%5Cr%5Cn%5Cr%5CnLet%27s%20just%20hear%20core%20developers%20thought%20about%20this.%5Cr%5Cn%22%2C%20%22created_at%22%3A%20%222014-08-22T06%3A29%3A19Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1428276%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/IronLady%22%7D%7D%2C%20%7B%22body%22%3A%20%22The%20spacing%20and%20concatenation%20is%20fine%20and%20good%20for%20readability%22%2C%20%22created_at%22%3A%20%222014-08-22T13%3A06%3A09Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1992427%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/ajeh%22%7D%7D%2C%20%7B%22body%22%3A%20%22If%20you%20really%20want%20readability%2C%20break%20it%20onto%20multiple%20lines%3A%5Cr%5Cn%60%60%60php%5Cr%5Cn%24trail_string%20.%3D%20%27%20%3Cspan%20itemscope%20itemtype%3D%5C%22http%3A//data-vocabulary.org/Breadcrumb%5C%22%3E%27%5Cr%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20.%20%27%3Ca%20itemprop%3D%5C%22url%5C%22%20href%3D%5C%22%27%20.%20%24this-%3E_trail%5B%24i%5D%5B%27link%27%5D%20.%20%27%5C%22%3E%27%5Cr%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20.%20%27%3Cspan%20itemprop%3D%5C%22title%5C%22%3E%27%20.%20%24this-%3E_trail%5B%24i%5D%5B%27title%27%5D%20.%20%27%3C/span%3E%27%5Cr%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20.%20%27%3C/a%3E%3C/span%3E%27%3B%5Cr%5Cn%60%60%60%22%2C%20%22created_at%22%3A%20%222014-08-22T13%3A39%3A04Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22For%20consistency%2C%20I%20think%20we%20shouldn%27t%20break%20it%20onto%20multiple%20line.%22%2C%20%22created_at%22%3A%20%222014-08-26T02%3A39%3A49Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1428276%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/IronLady%22%7D%7D%2C%20%7B%22body%22%3A%20%22I%20disagree.%20%20We%20should%20be%20%5BBoy%20Scouts%5D%28http%3A//www.c2.com/cgi/wiki%3FBoyScoutRule%29%20and%20%5Bfix%20broken%20windows%5D%28http%3A//www.c2.com/cgi/wiki%3FFixBrokenWindows%29.%22%2C%20%22created_at%22%3A%20%222014-08-26T05%3A49%3A19Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22I%20don%27t%20think%20that%20the%20code%20is%20broken.%20%5Cr%5CnI%20am%20just%20following%20the%20concatenation%20style%20on%20the%20original%20code%20which%20is%20the%20same%20with%20my%20concatenation%20style%20and%20think%20about%20version%20%3Ccode%3Eupgrade%3C/code%3E.%5Cr%5Cn%5Cr%5CnAnyway%2C%20Let%20just%20the%20others%20%28owner%29%20decide%20this.%5Cr%5Cn%5Cr%5CnCheers%20%3A%29%22%2C%20%22created_at%22%3A%20%222014-08-26T09%3A40%3A38Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1428276%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/IronLady%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20includes/classes/breadcrumb.php%3AL53-65%22%7D%2C%20%22Pull%20d51c4d706406a825b9f1313bfad3331d89e70a0c%20includes/classes/breadcrumb.php%2022%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/134%23discussion_r16437460%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Please%20restore%20the%20final%20new%20line.%22%2C%20%22created_at%22%3A%20%222014-08-19T19%3A29%3A00Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20includes/classes/breadcrumb.php%3AL72-76%22%7D%7D%7D'></a>
<a href='https://www.codereviewhub.com/'><img src='http://www.codereviewhub.com/site/github-bar.png' height=40></a>
- [ ] <a href='#crh-comment-Pull d51c4d706406a825b9f1313bfad3331d89e70a0c includes/classes/breadcrumb.php 8'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/zencart/zc-v1-series/pull/134#discussion_r16437458'>File: includes/classes/breadcrumb.php:L53-65</a></b>
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?v=2' height=16 width=16'></a> There are a couple concatenation breaks around the `span` which are unnecessary.
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?v=2' height=16 width=16'></a> Please remove the unnecessary concatenations around the `itemprop` span.
- <a href='https://github.com/IronLady'><img border=0 src='https://avatars.githubusercontent.com/u/1428276?v=2' height=16 width=16'></a> Thanks for the comment.
But, I don't think that it's unnecessary (the breaks).
I was thinking that I need to put one more break inside <code>itemscope</code> span.
```php
$trail_string .= '  <span itemscope itemtype="http://data-vocabulary.org/Breadcrumb">' . '<a itemprop="url" href="' . $this->_trail[$i]['link'] . '">' . '<span itemprop="title">' . $this->_trail[$i]['title'] . '</span>' . '</a>' . '</span>';
```
It can be something like a "mark" for us to easily edit it, IN CASE there are any schema markup changes in the future (who knows).
Let's just hear core developers thought about this.
- <a href='https://github.com/ajeh'><img border=0 src='https://avatars.githubusercontent.com/u/1992427?v=2' height=16 width=16'></a> The spacing and concatenation is fine and good for readability
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?v=2' height=16 width=16'></a> If you really want readability, break it onto multiple lines:
```php
$trail_string .= ' <span itemscope itemtype="http://data-vocabulary.org/Breadcrumb">'
. '<a itemprop="url" href="' . $this->_trail[$i]['link'] . '">'
. '<span itemprop="title">' . $this->_trail[$i]['title'] . '</span>'
. '</a></span>';
```
- <a href='https://github.com/IronLady'><img border=0 src='https://avatars.githubusercontent.com/u/1428276?v=2' height=16 width=16'></a> For consistency, I think we shouldn't break it onto multiple line.
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?v=2' height=16 width=16'></a> I disagree.  We should be [Boy Scouts](http://www.c2.com/cgi/wiki?BoyScoutRule) and [fix broken windows](http://www.c2.com/cgi/wiki?FixBrokenWindows).
- <a href='https://github.com/IronLady'><img border=0 src='https://avatars.githubusercontent.com/u/1428276?v=2' height=16 width=16'></a> I don't think that the code is broken.
I am just following the concatenation style on the original code which is the same with my concatenation style and think about version <code>upgrade</code>.
Anyway, Let just the others (owner) decide this.
Cheers :)
- [x] <a href='#crh-comment-Pull d51c4d706406a825b9f1313bfad3331d89e70a0c includes/classes/breadcrumb.php 22'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/zencart/zc-v1-series/pull/134#discussion_r16437460'>File: includes/classes/breadcrumb.php:L72-76</a></b>
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?v=2' height=16 width=16'></a> Please restore the final new line.
- [x] <a href='#crh-comment-General Comment'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/zencart/zc-v1-series/pull/134#issuecomment-52660891'>General Comment</a></b>
- <a href='https://github.com/IronLady'><img border=0 src='https://avatars.githubusercontent.com/u/1428276?v=2' height=16 width=16'></a> Sorry for the "reopened". Accidentally deleted my branch.
Thanks
- <a href='https://github.com/drbyte'><img border=0 src='https://avatars.githubusercontent.com/u/404472?v=2' height=16 width=16'></a> The improvements to richsnippet markup for breadcrumbs look good, and appear to pass the tests on Google's validator.
:+1:


<a href='https://www.codereviewhub.com/zencart/zc-v1-series/pull/134?mark_as_completed=1'><img src='http://www.codereviewhub.com/site/github-mark-as-completed.png' height=26></a>&nbsp;<a href='https://www.codereviewhub.com/zencart/zc-v1-series/pull/134?approve=1'><img src='http://www.codereviewhub.com/site/github-approve.png' height=26></a>&nbsp;<a href='https://github.com/zencart/zc-v1-series/pull/134'><img src='http://www.codereviewhub.com/site/github-refresh.png' height=26></a>
<a href='#crh-end'></a>
True, it's not "broken", per se.  But, we can still clean things up.  And, if we do a little here and a little there, it won't be long before there's not much to clean.  We shouldn't fall in the habit of bad code just to "fit in."

> On Aug 26, 2014, at 4:40 AM, Samuel Sena <notifications@github.com> wrote:
> 
> In includes/classes/breadcrumb.php:
> 
> >          } else {
> > -          $trail_string .= '  <a ' . ($i==($n-1) ? 'itemprop="url" ' : '') . 'href="' . $this->_trail[$i]['link'] . '">' . '<span itemprop="title">' . $this->_trail[$i]['title'] . '</span>' . '</a>';
> > +          $trail_string .= '  <span itemscope itemtype="http://data-vocabulary.org/Breadcrumb"><a itemprop="url" href="' . $this->_trail[$i]['link'] . '">' . '<span itemprop="title">' . $this->_trail[$i]['title'] . '</span>' . '</a></span>';
> I don't think that the code is broken. 
> I am just following the concatenation style on the original code which is the same with my concatenation style and think about version upgrade.
> 
> Anyway, Let just the others (owner) decide this.
> 
> Cheers :)
> 
> —
> Reply to this email directly or view it on GitHub.