When a user with no orders clicks on the 'addressbook' tab in their profile, the page is blank and no tabs appear (or messages)

same issue here

Status 'needs review' is for issues which actually have a patch.

I tried to reproduce this, but can't - and don't see a possible cause in the code either (note I didn't write this module, I just randomly wander through issue queues). Depending on permissions I either see an empty page with the expected tabs... or I see an "access denied" message for the page.

Hi, I'm getting the same error. I'll give the fix by @meeli a go in the short term.. just thought I'd +1 the issue.

Same issue on 7.x-2.0-rc7, @meeli tip fixed the issue (added an empty block to content region).
@roderik are you on the latest dev version that may explain it if you are (didn't get a chance to test it on dev).

@guy_schneerson: I tested 7.x-2.0-rc7. Couldn't reproduce.

...but while I confirmed the version number, I spotted something I didn't see before. user/*/addressbook returns a bogus value if $account->uid is not set, which leads to... probably the behavior you are encountering.

In includes/commerce_addressbook.user.inc:

/**
 * Access callback for path /user/%user/addressbook.
 *
 * Displays first enabled profile type addressbook page if user is logged in, or access denied for anonymous
 */
function commerce_addressbook_page($account) {
  if ($account->uid) {
    if ($type = commerce_addressbook_page_access($account)) {
      menu_set_active_item('user/' . $account->uid . '/addressbook/' . $type);
      return menu_execute_active_handler(NULL, FALSE);
    }
  }
  return FALSE;
}

First of all, the comment is wrong: this is a page callback, not an access callback.

And page callbacks may not return FALSE. The last line should read:
return MENU_ACCESS_DENIED;

Can you try that?

I think that will probably solve the "content region does not appear at all" issue, but it may give an 'access denied' message instead, and I don't know why it does that, in your case (instead of giving a page with an 'add address' link). But maybe that brings you one step closer to the solution.

Hi roderik Yes the comment is wrong but I dont think the code ever gets to return FALSE as both the user should be set and commerce_addressbook_page_access() returns a value.
Looks like the function changes the path and as a result the menu_execute_active_handler calls commerce_addressbook_profile_page() that returns an Empty string.
and its the issue of having no content on the page that's an issue.

If I replace the "return $output;" in commerce_addressbook_page_access() with:

<?php
  if ($output == '') {
    return '<div class="addressbook-noddata"></div>';
  }
  else {
   return $output;
  }
?>

the issue is fixed as we now have an empty div.

OK.

1)
I guess what you get, depends on the theme. For me (i.e. in the theme I tested), returning empty output would be fine. That is, I would get a normal 'empty' page with all the tabs rendered normally plus an "add new" link as a local action.

So I assumed that returning an empty string was 'legal' behavior. I can't find any documentation on that. But looking at it, drupal_set_page_content('') returns NULL, which I guess implies "we didn't account for this situation", i.e. setting an empty string as main page content is illegal.

Meaning, you're right.

2)

I actually did get a weird mangled page when I triggered the 'return FALSE'. Which I simply tested by visiting user/0/addressbook :)

(And I wrongly guessed that your problem with a mangled page came from your $account->uid being empty for some weird reason.)

Mmmmyeah. So the status I changed, is not valid anymore. If someone wants to roll a patch and get their name in the commit log, feel free, otherwise I guess I'll feel obliged to at some point, since I fiddled with someone's status :)

(Oh, and, also spotted yesterday: the access callback returns a string instead of just TRUE... which is also strange even though it doesn't matter in practice.)

Yes also got the weird mangled page when forcing a FALSE to return.
Will be happy to put in a patch and will replace the return FALSE.
Will also change the empty div with a bit of helpful text "Your address book is currently empty."

Attached patch adds an empty address book message, it also cleans up the comments and replaces the return FALSE.

Nice.

Changes:

* retutn -> return in comment

* My IDE reminded me that in the first message, $profile_type does not exist. (And $type is empty too.)

* I don't see the reason for doing 'addressbook-noddata', can we name it 'addressbook-nodata'?

Hi @roderik

ow yes just copied the line from commerce_addressbook_profile_page() and missed out that we don't have the $profile_type in commerce_addressbook_page() - thank your IDE from me.

and "noddata" is a typo should be 'nodata' or probably 'addressbook-no-data' best.

Can update the patch tomorrow

I have, please review patch #12

ow yes sorry roderik yes looks perfect.
Can't see anything wrong with the patch, great work, will try and test it later, but I am sure it will work.

just tested and works perfectly also looked at the interdiff and all the changes are as you described and make perfect sense.

Patch in #12 works perfect fo me too.

Thanks.

Patch in #12 worked perfectly here too :)