When viewing a node draft or it's current revision, the standard drupal menu_get_object() function is returning the published node instead of the revision thats actually being viewed. This is inconsistent with the revision-specific URLs (which can be clicked from the Moderate tab for a node), where the specific revision is loaded by menu_get_object().

The core of the issue is that the draft and current-revision URLs use %node as a replacement value in their paths, but don't have a % wildcard for the revision. %node triggers drupal into loading the menu object using the node_load() function, which can accept both a $nid as the first argument and a $vid as the second argument. With the revision-specific URLs this allows the specific node revision to be loaded, since the path has a second % wildcard for the revision number. But with the draft and current-revision URLs, which have no value for the revision number, node_load is called with only a $nid, and thus the published version is retrieved instead of the version being viewed.

This caused an issue on my site because we were executing code based on the node being viewed, and we assumed that we could depend on menu_get_object(). I have since written a workaround on our site, using a function to wrap menu_get_object() and calling that instead.

I did take a shot at fixing this in the module, but I'm not sure there is an easy way to do so. My first attempt was to change the load function from node_load to workbench_moderation_node_load by changing the wildcard in the draft and current-revision URLs to %workbench_moderation_node. However, this causes the if clause in menu_get_object() to fail when it is looking for "$router_item['load_functions'][$position] == $type . '_load'". Translated, the values are "workbench_moderation_node_load" == "node_load", which is obviously FALSE.

I then tried a terrible horrible hack in hook_node_load (specifically workbench_moderation_node_load) where I analyzed the request path and PHP backtrace to see if menu_get_item was being called for the current node, and hacked it correct in there. Not only was it a terrible hack (that did indeed work), but it also puts an incorrect value in the static cache for node_load, meaning any subsequent calls in the request to node_load() with just the $nid will return the draft instead of the published version.

Perhaps someone has a better idea on how to go about this?

CommentFileSizeAuthor
#19 2021903-workbench_moderation-menu_get_object-fix-19.patch667 bytesmanishmore
#16 2021903-workbench_moderation-menu_get_object-fix.patch667 bytesbeeradb
#4 workbench_moderation-draft_current_revision-2021903.patch0 bytesbjmiller121
#5 workbench_moderation-draft_current_revision-2021903.patch671 bytesbjmiller121

Comments

bjmiller121’s picture

bjmiller121 commented

I'm having this same issue with the cck_blocks module that creates a block out of a cck field to be included elsewhere on the page. Here's a fix to another issue with cck_blocks if other people are trying to get cck_blocks to work with workbench_moderation: #2058139: CCK blocks don't show up on draft pages

This is kind of a big hang-up on using this module because the site I'm working on very heavily uses CCK blocks and it would mean that the draft view is not reliable as a representation of the draft content in most cases.

Seems like the best solution would be to use the same path structure for /draft /current-revision as core revisions by including the $vid in the path:

node/{nid}/revisions/{vid}/view

I'll take a stab at a patch (tiny bit over my head here), but would you mind sharing your workaround? I might be willing to just patch the cck_blocks module calling menu_get_object() for now in order to get this to work on my site.

brettbirschbach’s picture

brettbirschbach commented

I just made my own version of the function to get the node for the current page.

function my_module_get_page_node() {
  $node = &drupal_static(__FUNCTION__);
  if (!isset($node)) {
    if ($node = menu_get_object()) {
      if (module_exists('workbench_moderation')) {
        $request_path = request_path();
        $current_paths = array('/^node\/(\d+)\/draft/', '/^node\/(\d+)\/current-revision/');
        foreach ($current_paths as $path) {
          if (preg_match($path, $request_path)) {
            $node = workbench_moderation_node_current_load($node);
          }
        }
      }
    }
    else {
      $node = FALSE;
    }
  }
  return $node ? $node : NULL;
}

Unfortunately if you have a contrib module that needs to use this workaround, you'd have to update calls of menu_get_object() to use this function instead.

bjmiller121’s picture

bjmiller121 commented

So here's one way of getting the correct revision passed to menu_get_object. Seems like there might be a better way, but my other approach kept running into the same issues you mentioned.

Essentially, we're just setting the node object stored in menu_get_item to be the current node rather than the published node before menu_get_object returns the node.

bjmiller121’s picture

bjmiller121 commented
StatusFileSize
new workbench_moderation-draft_current_revision-2021903.patch0 bytes

Sorry, see patch in #5

bjmiller121’s picture

bjmiller121 commented
StatusFileSize
new workbench_moderation-draft_current_revision-2021903.patch671 bytes

Sorry, not sure why that patch didn't work. This one should work.

I'm new to this. :P

jimsmith’s picture

jimsmith commented

I tested this patch. It seems to fix what it was intended to fix, but I'm now finding other problems with Workbench Moderation after fixing this problem. Namely, when using Pathauto there are redirect issues, including an infinite loop. Those problems have been reported in a number of places, such as Infinite redirect with Workbench Moderation module and Redirects incorrectly created when saving as draft (using pathauto/redirect default settings).

fischeme’s picture

fischeme commented

I have had this same issue when using revisioning without workbench moderation at all, so I wonder if the real problem is in revisioning. Basically, when I set revisioning to redirect to the latest revision when a node is accessed (it is a config option in revisioning), the latest pending revision displays but menu_get_object() returns the current revision. So, all my logic that depends on that function operates on the wrong node. Logged in: https://www.drupal.org/node/2344619

ladybug_3777’s picture

ladybug_3777 commented

I'm trying out the function provided by HitmanInWis in comment #2. So far this is working as I expect. I like that it gives me the control to call the function on my terms when I know menu_get_object may fail.

Thanks!

eric_a’s picture

eric_a commented
Version: 7.x-1.3 » 7.x-1.x-dev
Status: Active » Needs review

The last submitted patch, 5: workbench_moderation-draft_current_revision-2021903.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 4: workbench_moderation-draft_current_revision-2021903.patch, failed testing.

Eric_A queued 5: workbench_moderation-draft_current_revision-2021903.patch for re-testing.

The last submitted patch, 5: workbench_moderation-draft_current_revision-2021903.patch, failed testing.

Eric_A queued 5: workbench_moderation-draft_current_revision-2021903.patch for re-testing.

eric_a’s picture

eric_a commented
Status: Needs work » Needs review
beeradb’s picture

beeradb commented
StatusFileSize
new 2021903-workbench_moderation-menu_get_object-fix.patch667 bytes

The previous patch contained coding standards issues. Here's a re-roll with the issues fixed.

mariacha1’s picture

mariacha1 commented
Status: Needs review » Reviewed & tested by the community

Patch works for me!

autopoietic’s picture

autopoietic commented

Patch in comment #16 works for me also

manishmore’s picture

manishmore commented
StatusFileSize
new 2021903-workbench_moderation-menu_get_object-fix-19.patch667 bytes

Patch worked for me too, updated to latest version of it.

nicrodgers’s picture

nicrodgers
Primary language English
Location Monmouthshire, UK
commented

@manishmore, the patch you uploaded in #19 appears identical to the patch in #16. Did you mean to upload it?

capellic’s picture

capellic
he/him/his
Location Austin, Texas
commented

Is this still in an issue in the 3.x branch? Yes, yes it is. This patch is needed for 3.x

capellic’s picture

capellic
he/him/his
Location Austin, Texas
commented
Version: 7.x-1.x-dev » 7.x-3.x-dev

  • das-peter committed 11fdb6f on 7.x-3.x authored by bjmiller121 
    Issue #2021903 by bjmiller121, beeradb, manishmore: menu_get_object does...
das-peter’s picture

das-peter commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

devaraj johnson’s picture

devaraj johnson commented

Any plan for a new release with the above patch fixed in 7.3

le72’s picture

le72
Location Yerevan 🇦🇲
commented

+1 to add to release.

kunalkursija’s picture

kunalkursija commented

+1 for this to be in a release.

emersonreis.dev’s picture

emersonreis.dev
he/him/his
Primary language Portuguese, Brazil
Location Balneário Camboriú
commented

+1 for a release. worked for me too.