error for menu.inc

Yep, known issue. I think it's related to the half done port of mark as read.

Michelle

Also reverting back to the previous dev build will resolve

True, but that's not really any more usable. That's why the project page says not to use it. ;) It's getting there... I just need some more time to work on it.

Michelle

I commented out the advanced_forum_menu() function, and the message went away. Everything else appears to work fine. But I have absolutely no idea what I am doing, so I will probably find something really broken later.

Where did you do that? I tried it, and it did not help :(

subscribe

Somehow it dissapeared today .. dont ask me how :(.

I woke up this morning, started my Server and this failure was just gone ...

Edit: Just realised that the "Mark all forums as read" link is gone also. Well I dont need it so its no Problem but maybe a hint?

Considering it's the code for mark all read that's causing the problem, I'd say there's a definite connection.

Michelle

JThan: the function is in the .module file. Also, the reason it didn't work for you right away was likely due to the cacheing feature. Clearing the cache would have fixed it right away. But since you waited, the cache expired on it's own, ultimately achieving the same result.

Yes, removing the function removed the link -- that's the side-effect of this workaround. I can live with that until Michelle has the time to do her magic... Perhaps I'll get brave enough and actually figure out the root cause, and perhaps I'll even go as far as to post a patch. But I'm too busy already to be a Drupal developer :)

Well, I did try it yesterday, but when it was not working undoing the changes was first thing to do.
Just again checked the .module file... the function is not marked out.

So lets see what happens tomorrow

The reason for the error is that the parameters for access in hook_menu are no longer user_access && whatever you now pass 'user_access' in 'access callback' and define the parameters in 'access arguments'. Except... It doesn't like it if you want to define things like $user->uid into the mix as well. They changed it because sometimes modules need that extra flexibility of being able to define complex access rules themselves (in their own function) - there's info about it here; http://drupal.org/node/109157

As a suggestion, I'd tackle the issue like this;

/**
 * Implementation of hook_menu.
 */
function advanced_forum_menu() {
  $items['forum/markasread'] = array(
    'title' => 'Helper page to mark forums read.',
    'page callback' => 'advanced_forum_markasread',
    'access callback' => 'advanced_forum_markasread_access',
    'type' => MENU_CALLBACK,
  );

   return $items;
}

function advanced_forum_markasread_access() {
  global $user;
  return user_access('access content') && $user->uid;
}

Defining my own callback function for access and returning a TRUE or FALSE for access.

Pobster

I know the reason for the error and I've already fixed it, which is why I marked it fixed. Thanks for your efforts, but I don't think breaking it into a separate function is needed.

Michelle

Apologies, but then you did only commit that 15 minutes ago...

Actually... I've just taken a look at your change;

function advanced_forum_menu() {
  global $user;
  if ($user->uid) { 
    $items['forum/markasread'] = array(
      'title' => 'Helper page to mark forums read.',
      'page callback' => 'advanced_forum_markasread',
      'access callback' => 'user_access',
      'access arguments' => array('access content'),
      'type' => MENU_CALLBACK,
    );
  }

I'm not sure that's valid? It would have been under 5.x but in 6.x I think the menu system isn't checked the same as it used to be. I'll test the theory once the nightly build has become available for download, but I believe the menu hook is only called when the cache is empty and hence if it's built when any user isn't logged in the markasread link won't show for anyone. I'm pretty sure the menu is cached with the access callback/ access arguments and the user is 'tested' for permissions from the cached menu conditions not by looking over the menu hook again. Like I said though, I'll test... But I'm 99% sure the change you've made will have this problem. That's kind of the reason we don't pass $may_cache anymore and have the whole hook_init/ hook_boot thing.

Pobster

I fixed it last night but was pulled away before I had the chance to commit my latest changes. I mark things fixed when I fix them. It's not normally an issue because I usually commit right away. Last night was an exception.

Anyway, if I'm understanding you right, you're saying this isn't going to work if the cache gets emptied when no one is logged in? That's a rather wierd edge case, but I can test it and see.

Thanks,

Michelle

Yeah as I understand it, the new menu system only runs through hook_menu to populate the menu cache. So if when it's built the user who causes the hook to be called isn't logged in then you're conditioning out the $item containing the markasread link. How it's supposed to work is that the menu_hook contains literally only menu items, and the menu cache holds all the condition caveats to be looked at. This is why you pass an array to 'access arguments' rather than the old user_access('whatever') as the new menu system is trying to move away from having over-busy, over-complicated menu hooks. As I said though, that's only how I understand it...

There's hinting of it here; http://api.drupal.org/api/function/hook_menu

"This hook is called rarely - for example when modules are enabled."

Pobster

Actually, there seems to be the opposite problem. It's not denying access for anon, either.

Anyway, this issue is fixed. This was about getting rid of the error, which I did. I haven't finished making the whole mark as read thing working but there's a separate issue for that here: http://drupal.org/node/241071

Michelle

Just wanted to add that I do appreciate you taking the time to explain this. I just realized my pickiness about keeping issues on topic is making me look ungrateful. I don't want you to think you're wasting your time.

Michelle

Meh I'm not so bothered about getting the mark as read function working, I was only like the rest of this thread trying to remove the error from hook_menu which was caused by the menu system unserializing the broken access conditions, which as you've said, you've changed now...

However... As I said I'm 99% sure what I'm telling you (and trying desperately to provide evidence for) that your change is going to cause intermittent issues for your users. Intermittent because the menu hook is, as I quoted above, only run on rare occasions such as when the cache is emptied. Don't forget that Drupals new menu system is drastically different from the old version and so the old rules don't apply any more, I've had many problems with it myself with my own modules - I'm just trying to avoid this being a problem for you? I'm only trying to avert a possibly hard to track down/ replicate issue you'll likely have in the future...

Pobster

Lol ;o) No, no - it's quite okay, you don't know me from Adam and I quite understand your guardedness regarding the issue. Especially as well... It's hard to replicate! I mean, it's not like I'm even having any problems myself - I've just played with the Drupal 6.x menu system until I've been blue in the face and although most of the changes are for the better, the completely different way it's done now is so very confusing when I was so used to the old system.

I just wish I could explain the issue a bit better! It's essentially that as hook_menu is called rarely, it's built on *any* user who 'activates' it. Hence if they're not logged in, the $user->uid condition will exclude the markasread menu item from the menu cache entirely - it needs to be included and have access conditions available for it to check in that cache not from within the hook. Hmmm, actually that's explained it a whole lot better! Hope this helps!

Pobster

"I'm only trying to avert a possibly hard to track down/ replicate issue you'll likely have in the future..."

And I appreciate that. Now that I understand what you're saying, your original change makes more sense. I started out grumpy because I've repeatedly said the 6.x branch isn't ready for use, yet, and I woke up (after too little sleep) to someone unfixing my issue because I wasn't able to commit before bed. But, then, I took a step back and realized you were actually giving me useful information that I would likely have not figured out on my own, at least not easily, and, so, I do apologize for my initial grumpiness.

I put a pointer in the mark as read issue to your fix.

Thanks,

Michelle

[Edit: We keep posting over the top of each other. It's ok. I'm pretty sure I understand the issue, now. I'll work on it soon as this baby gives me some peace. :)]

Sorry it's mostly my fault for not having the words to be able to explain myself clearly enough in the first place!

No harm done!! ;o)

Pobster

Just an FYI - I ended up using your suggestion and it came in handy later when I needed to hide the link from those who can't access it. I apologize for leaving you off the commit message. Been so busy fighting with the actual MAR function all day that I forgot about that bit of code that I added this morning until just now.

Thanks,

Michelle

Absolutely not a problem - just glad to help! ;o)

Pobster