As I was trying to get the canonical meta tag to work correctly on one of my sites, I found out that you had a "return" in the globalredirect_init() function in case you're handling the front page.

A skeleton of the code goes like this:

  if ( front page ) {
    if ( ! allow front page redirects ) {
      return
    }
    handle front page
    return
  }
  handle all other pages

  // then...
  create canonical meta tag
  create content location meta tag

As you can see, if you return when handling the front page special cases, then you never get to the canonical and location meta tag code...

The proposed change looks huge because of the indentation change (apply the patch and try a diff with your current version using the -B flag to avoid seeing the indentation change and you'll see that code wise the change is very small.)

  if ( front page ) {
    if ( allow front page redirects ) {
      handle front page
    }
  }
  else {
    handle all other pages
  }
  create canonical meta tag
  create content location meta tag

As we can see, we don't return and we use the powerful "else" instruction to jump to the right place.

Of course, a cleaner way to do this is to have a sub-function to handle the redirections and call it. On return you continue with the canonical and location meta tags... then the if/then/else is hidden in the sub-function which could even use the return keyword.

Thank you.
Alexis

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, globalredirect-metatags-6.x.patch, failed testing.

AlexisWilke’s picture

Trying again with a git patch. 8-P

AlexisWilke’s picture

Status: Needs work » Needs review

Need to ask for a re-review of course...