Launchpad itself

Merge lp://staging/~mbp/launchpad/meta-description into lp://staging/launchpad

meta-description
Merge into devel

Proposed by Martin Pool on 2011-11-19

Status:	Merged
Approved by:	Martin Pool on 2011-11-22
Approved revision:	no longer in the source branch.
Merged at revision:	14362
Proposed branch:	lp://staging/~mbp/launchpad/meta-description
Merge into:	lp://staging/launchpad
Diff against target:	336 lines (+139/-17) 11 files modified lib/canonical/launchpad/webapp/publisher.py (+18/-0) lib/lp/app/browser/stringformatter.py (+12/-0) lib/lp/app/templates/base-layout.pt (+4/-0) lib/lp/bugs/browser/bug.py (+4/-0) lib/lp/bugs/browser/bugtask.py (+4/-0) lib/lp/bugs/browser/tests/test_bug_views.py (+38/-13) lib/lp/code/browser/tests/test_branchmergeproposal.py (+17/-0) lib/lp/registry/browser/person.py (+10/-0) lib/lp/registry/browser/tests/test_person_view.py (+21/-0) lib/lp/services/utils.py (+7/-3) lib/lp/testing/factory.py (+4/-1)
To merge this branch:	bzr merge lp://staging/~mbp/launchpad/meta-description
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Raphaël Badin (community)		2011-11-19	Approve on 2011-11-21
Review via email: mp+82769@code.staging.launchpad.net

Commit message

[r=rvb][no-qa] add meta description for bugs and bmps

Description of the change

Google sometimes generates lame page summaries for Launchpad pages, containing garbage that occurs near the start of the page.

We can give it a better clue by putting in meta description tags.

For an actual search it will often but not always generate its own summary looking at the page text, but the meta description is sometimes used there. The meta description seems to be very often used when sharing a link on g+ and facebook and at the moment it looks awful.

See:

http://googlewebmastercentral.blogspot.com/2007/09/improve-snippets-with-meta-description.html

g+ before: https://plus.google.com/112646476239496153808/posts/bMVdVYUP6XA
after: https://plus.google.com/112646476239496153808/posts/BRxTsjaUeWu

fb before: https://www.facebook.com/martin.pool/posts/107832539332513
after: https://www.facebook.com/martin.pool/posts/119663848146056

(apparently you must log in even to see fb "public" posts)

hooray!

We could do this for other pages too, but we can start with bugs and merge proposals.

Revision history for this message

Martin Pool (mbp) wrote on 2011-11-19:

If there's a more zopey way to write this, let me know. I was going to try to make all view objects provide a page_description property, but I was defeated.

Revision history for this message

Martin Pool (mbp) wrote on 2011-11-20:

I worked out how to do it with less duplication in the templates: view classes can now just provide a description.

Revision history for this message

Martin Pool (mbp) wrote on 2011-11-21:

of all of these metadata patches, this is probably the most sure to have a beneficial effect

Revision history for this message

Raphaël Badin (rvb) wrote on 2011-11-21:

This looks good to land.

[0]

224 + expected_meta = Tag(
225 + 'meta description',
226 + 'meta', attrs=dict(
227 + name='description',
228 + content=True))

Don't you want to test for the presence of the (potentially cropped) 'description' ?

[1]

243 + def page_description(self):
244 + context = self.context
245 + if context.is_valid_person_or_team:
246 + return (
247 + self.context.homepage_content
248 + or self.context.teamdescription)
249 + else:
250 + return None

Don't you want to return u'' on line 250 here?

[2]

281 + self.assertThat(view.page_description,
282 + Equals(person_description))

A fascist reviewer would ask you to put a new line before "view.page_description," ;)

[3]

300 + self.assertEqual(
301 + None, view.page_description)

I think you can save a line here.

review: Approve

Revision history for this message

Martin Pool (mbp) wrote on 2011-11-22:

Thanks for the review.

I will test the actual content.

On consideration, I think I will strip email addresses always, since the description is commonly used by sharing tools and they shouldn't share the email address, even when started by an authenticated user.

> Don't you want to return u'' on line 250 here?

No I think I do want None, so the description just doesn't appear.