Merge lp:~mbp/launchpad/888353-microformats into lp:launchpad

[Martin Pool]

[r=rvb][bug=888353] add schema.org microformats for bugs and breadcrumbs

This adds a small amount of schemas.org to the main bug page and to the generic breadcrumbs, with a goal of getting search engines to give better summaries snippets of bug pages that turn up in search results.

To see whether it actually does work or not we'll need to deploy this and wait for it to be indexed. We could do a lot more but this is an experiment to see if it has a noticeable effect.

There are not any (intentional) changes is normal page content or behaviour.

You can see Google's test parser on it at <http://www.google.com/webmasters/tools/richsnippets?url=http%3A%2F%2Fsourcefrog.net%2Ftmp%2F1w%2Fbug1.html&view=cse>.

[Raphaël Badin (rvb)]

Hi Martin, this looks like a very good idea but I've got a few remarks. I'm no microformat expert so please bear with me ;).

= Real questions/remarks =

[0]

8 + itemscope itemtype="http://schema.org/WebPage"

You only include the breadcrumbs here, don't you think it would be useful (for the test to be complete), to also include a name (http://schema.org/Thing) and a URL (http://schema.org/Thing) [and maybe a description (http://schema.org/Thing)]? Obviously the parser can take the url as an identifier but maybe it's better if we explicitly provide one.

[1]

38 + itemprop="commentTime"

I'm not sure "on XXX" is correctly parsed as a date, and I'm pretty sure "a moment ago" won't be. Maybe it would be worth to add the date properly formatted in the tag with an attribute such as """ datetime="2007-10-20" """.

[2]

I cannot help but notice that there is no test for this. I understand this is a test so it's probably ok… Maybe you could include an empty (or almost empty) test for this with a reference to this bug just so that we don't forget to add tests should we develop this further.

= Details =

[0]

37 <span tal:attributes="title comment/datecreated/fmt:datetime"
38 + itemprop="commentTime"
39 tal:content="comment/datecreated/fmt:displaydate">

Weird formatting here.

[1]

45 <td class="bug-comment-index">
46 - <a tal:attributes="href comment/fmt:url"
47 - tal:content="string: #${comment/index}" />
48 + <a itemprop="url"
49 + tal:attributes="href comment/fmt:url"
50 + tal:content="string: #${comment/index}" />

And here.

review: Approve

[Martin Pool (mbp)]

2011/11/21 Raphaël Badin <email address hidden>:
> Review: Approve
>
> Hi Martin, this looks like a very good idea but I've got a few remarks.  I'm no microformat expert so please bear with me ;).

Thanks for being willing to review it anyhow.

>
> = Real questions/remarks =
>
> [0]
>
> 8       +    itemscope itemtype="http://schema.org/WebPage"
>
> You only include the breadcrumbs here, don't you think it would be useful (for the test to be complete), to also include a name (http://schema.org/Thing) and a URL (http://schema.org/Thing) [and maybe a description (http://schema.org/Thing)]? Obviously the parser can take the url as an identifier but maybe it's better if we explicitly provide one.

Putting in the description, probably using the same thing for the meta
description, might be useful. (Though, also kind of redundant
perhaps.)

Putting the URL for the page itself seems a bit redundant too, but
that could go off the canoncial_url_recommended.

A place where I can imagine the url being useful is in eg comments on
a bug, which each have a scope and a url of their own.

To some extent we're guessing how the black box of search engines will
behave - in the first cut I'm trying to do something large enough we
may see some difference without spending too much time or making too
intrusive changes.

>
> [1]
>
> 38      +                 itemprop="commentTime"
>
> I'm not sure "on XXX" is correctly parsed as a date, and I'm pretty sure "a moment ago" won't be.  Maybe it would be worth to add the date properly formatted in the tag with an attribute such as """ datetime="2007-10-20" """.

Good point.

>
> [2]
>
> I cannot help but notice that there is no test for this.  I understand this is a test so it's probably ok…  Maybe you could include an empty (or almost empty) test for this with a reference to this bug just so that we don't forget to add tests should we develop this further.

Good idea too.

--
Martin

[Raphaël Badin (rvb)]

> A place where I can imagine the url being useful is in eg comments on
> a bug, which each have a scope and a url of their own.

Right.

> To some extent we're guessing how the black box of search engines will
> behave - in the first cut I'm trying to do something large enough we
> may see some difference without spending too much time or making too
> intrusive changes.

True… I'm really looking forward to see how this impacts Goo^Wsearch engines. I'm subscribed to the related bug… would you be kind enough to post a small summary of your findings in the bug comments when you'll have assessed the impact of this change?

[Martin Pool (mbp)]

ok, this update
 * changes to a <time datetime=> tag
 * uses a machine readable date
 * adds a test for that tag

Is there a better way to write this test?

thanks.

[Raphaël Badin (rvb)]

> ok, this update
> * changes to a <time datetime=> tag
> * uses a machine readable date
> * adds a test for that tag
>
> Is there a better way to write this test?
>
> thanks.

It's a little bit verbose, I'll give you that… but fairly readable still I think.

=== modified file 'lib/lp/app/doc/hierarchical-menu.txt'
--- lib/lp/app/doc/hierarchical-menu.txt	2011-05-27 18:27:59 +0000
+++ lib/lp/app/doc/hierarchical-menu.txt	2011-11-24 01:04:28 +0000
@@ -248,7 +248,7 @@
 represents the current location.
 
     >>> print markup
-    <ol class="breadcrumbs">
+    <ol itemprop="breadcrumb" class="breadcrumbs">
       <li>
         <a href="http://launchpad.dev/joy-of-cooking">Joy of cooking</a>
       </li>

=== modified file 'lib/lp/app/templates/base-layout.pt'
--- lib/lp/app/templates/base-layout.pt	2011-11-20 06:38:50 +0000
+++ lib/lp/app/templates/base-layout.pt	2011-11-24 01:04:28 +0000
@@ -62,6 +62,8 @@
   </head>
 
   <body id="document"
+    itemscope=""
+    itemtype="http://schema.org/WebPage"
     tal:attributes="class string:tab-${view/menu:selectedfacetname}
       ${view/macro:pagetype}
       ${view/context/fmt:public-private-css}

=== modified file 'lib/lp/app/templates/launchpad-hierarchy.pt'
--- lib/lp/app/templates/launchpad-hierarchy.pt	2010-07-21 18:03:44 +0000
+++ lib/lp/app/templates/launchpad-hierarchy.pt	2011-11-24 01:04:28 +0000
@@ -1,4 +1,5 @@
 <ol
+  itemprop="breadcrumb"
   class="breadcrumbs"
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"

=== modified file 'lib/lp/bugs/browser/tests/test_bugcomment.py'
--- lib/lp/bugs/browser/tests/test_bugcomment.py	2011-11-16 23:46:01 +0000
+++ lib/lp/bugs/browser/tests/test_bugcomment.py	2011-11-24 01:04:28 +0000
@@ -15,6 +15,14 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from soupmatchers import (
+    HTMLContains,
+    Tag,
+    )
+
+from canonical.launchpad.ftests import (
+    login_person,
+    )
 from canonical.launchpad.testing.pages import find_tag_by_id
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -287,3 +295,24 @@
         naked_bugtask = removeSecurityProxy(context.default_bugtask)
         removeSecurityProxy(naked_bugtask.pillar).security_contact = person
         self._test_hide_link_visible(context, person)
+
+
+class TestBugCommentMicroformats(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_bug_comment_metadata(self):
+        owner = self.factory.makePerson()
+        login_person(owner)
+        bug_comment = self.factory.makeBugComment()
+        browser = self.getViewBrowser(bug_comment)
+        iso_date = bug_comment.datecreated.isoformat()
+        self.assertThat(
+            browser.contents,
+            HTMLContains(Tag(
+                'comment time tag',
+                'time',
+                attrs=dict(
+                    itemprop='commentTime',
+                    title=True,
+                    datetime=iso_date))))

=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt	2010-03-22 22:56:44 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt	2011-11-24 01:04:28 +0000
@@ -30,14 +30,14 @@
     ...         print div_tag
     >>> print_comments(browser.contents) #doctest: -ELLIPSIS
     <div class="boardCommentBody">
-    <div class="bug-comment"><p>This
+    <div class="bug-comment" itemprop="commentText"><p>This
     would be a real killer feature. If there...</p></div>
     <p>
     <a href="/tomcat/+bug/2/comments/1">Read more...</a>
     </p>
     </div>
     <div class="boardCommentBody">
-    <div class="bug-comment"><p>Oddly
+    <div class="bug-comment" itemprop="commentText"><p>Oddly
     enough the bug system seems only capabl...</p></div>
     <p>
     <a href="/tomcat/+bug/2/comments/2">Read more...</a>
@@ -57,7 +57,7 @@
 
     >>> print_comments(browser.contents) #doctest: -ELLIPSIS
     <div class="boardCommentBody">
-    <div class="bug-comment"><p>This
+    <div class="bug-comment" itemprop="commentText"><p>This
       would be a real killer feature. If there is already code
       to make it possible, why aren't there tons of press announcements
       about the secuirty possibilities. Imagine - no more embarrassing

=== modified file 'lib/lp/bugs/templates/bugcomment-box.pt'
--- lib/lp/bugs/templates/bugcomment-box.pt	2011-09-01 10:54:38 +0000
+++ lib/lp/bugs/templates/bugcomment-box.pt	2011-11-24 01:04:28 +0000
@@ -2,6 +2,8 @@
   xmlns="http://www.w3.org/1999/xhtml"
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
+  itemscope=""
+  itemtype="http://schema.org/UserComments"
   tal:define="comment context;
               remote_bug_comment_class
               python: comment.bugwatch and 'remoteBugComment' or '';
@@ -19,10 +21,13 @@
             <tal:comment_owner
                 replace="structure comment/owner/fmt:link-display-name-id" />
             wrote
-            <span tal:attributes="title comment/datecreated/fmt:datetime"
-                  tal:content="comment/datecreated/fmt:displaydate">
-              7 minutes ago
-              </span>:
+            <time
+              itemprop="commentTime"
+              tal:attributes="title comment/datecreated/fmt:datetime;
+                datetime comment/datecreated/fmt:isodate"
+              tal:content="comment/datecreated/fmt:displaydate">
+                7 minutes ago
+            </time>:
               <a tal:attributes="href comment/fmt:url"
                  tal:condition="comment/display_title">
                 <strong tal:content="comment/title" />
@@ -46,8 +51,9 @@
               </a>
           </td>
           <td class="bug-comment-index">
-            <a tal:attributes="href comment/fmt:url"
-               tal:content="string: #${comment/index}" />
+	  <a itemprop="url"
+	     tal:attributes="href comment/fmt:url"
+             tal:content="string: #${comment/index}" />
           </td>
         </tr>
       </tbody>
@@ -79,7 +85,7 @@
       </li>
     </ul>
 
-    <div class="bug-comment"
+    <div class="bug-comment" itemprop="commentText"
       tal:content="structure
         comment/text_for_display/fmt:obfuscate-email/fmt:email-to-html">
       Comment text.

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2011-11-14 04:07:15 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2011-11-24 01:04:28 +0000
@@ -129,7 +129,7 @@
         <div><!-- id="nonportlets"> -->
         <div class="top-portlet">
 
-      <div class="report">
+      <div itemprop="mainContentOfPage" class="report">
         <tal:description
             define="global description context/bug/description/fmt:obfuscate-email/fmt:text-to-html" />