Node View Task Handler Doesn't Include a Canonical URL

I didn't even know about this feature. Neat. Committed, pushed.

Commit 05d7202ef562b90fa1cf8c541ac3ef0b35077613 removed this code, making our canonical links to be broken...

Added patch for 7.x-1.9.

Added a test for this.

The test in #5 always succeeds, even without the patch. This was because the node that was created by the test was not handled by Page manager at all.
I fixed that, plus some coding standards and the fact that it was not working in sub-directory installation. I upload interdiff, tests only and the new patch.

Patch applies properly, and it has tests :-) and the tests run fine and fail when the changed code is reverted.

I am assuming that the change itself is a good one to make - it seems so, but there is a niggle in my head that changing existing behaviour (however broken) in this way might not be good. If someone had worked around the failure would they need to change? If so, I think we need to flag this in the release notes, at the very least.

There are a couple of review comments, but nothing serious. Proposing RTBC for 1.13 if we can get the above sorted.

1. +++ b/page_manager/plugins/tasks/node_view.inc
   @@ -85,6 +85,12 @@ function page_manager_node_view_page($node) {
   +  $uri = entity_uri('node', $node);
   +  // Set the node path as the canonical URL to prevent duplicate content.
   +  drupal_add_html_head_link(array('rel' => 'canonical', 'href' => url($uri['path'], $uri['options'])), TRUE);
   +  // Set the non-aliased path as a default shortlink.
   +  drupal_add_html_head_link(array('rel' => 'shortlink', 'href' => url($uri['path'], array_merge($uri['options'], array('alias' => TRUE)))), TRUE);
   +
   

   Two thoughts about these lines:

   - is it possible in this case for entity_uri() to return something other than the expected array? if so we need to check $uri before use;

   - the lines are long and would benefit from being reformatted onto multiple lines.

2. +++ b/page_manager/tests/head_links.test
   @@ -0,0 +1,74 @@
   +class HeadLinksTestCase extends DrupalWebTestCase {
   

   Yay!! someone providing test coverage :) thanks.

3. +++ b/page_manager/tests/head_links.test
   @@ -0,0 +1,74 @@
   +      'group' => 'Chaos Tools Suite',
   

   Recent changes have settled on 'ctools' for the test group.

Ok, I have implemented a change to #6 which addresses my review comments; I hope the changed code in node_view.inc is appropriate.

I have also modified the test file group and added descriptor strings to two asserts.

It should be noted that the tests intentionally create a 404 not found error; this is not a bug!

Looks good to me, and love the test. Committed!