Active
Project:
Workbench Moderation
Version:
7.x-1.3
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
11 Mar 2011 at 22:22 UTC
Updated:
13 Feb 2014 at 20:05 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
mechler commentedI have two imperfect ways to fix this, and the real solution is unclear to me. While I keep digging, here are the two methods that at least get you back to working without diff.
Note: The only use cases that I'm exploring here involve workbench_moderation NOT being used to manage the content type. With workbench_moderation enabled for the content type, we never get this error.
issue1089834-r1.patch
Here we include the /modules/node.pages.inc file because node's hook_menu() implementation doesn't specify a 'file_path' because the path for where it's going to find it's 'file' is implicit. Changing the 'page callback' in workbench_moderation's hook_menu_alter() implementation means that we lose track of where the files are that were previously going to be included.
This has the benefit of never providing an error. The problem -- aside from inelegance -- is that while it doesn't cause an error, it also breaks diff so that it never loads the diff_diffs_overview() but instead just keeps using core's revisioning system.
issue1089834-r2.patch
The alternative method is to explicitly tell the menu_router_item where to be looking for node.pages.inc (/modules/node). The problem here is that the diff module breaks and spits out an error when you try to view the revisions with the diff module enabled. The reason for this is similar. We tell the router item to include 'node.pages.inc' and (with this patch) where to find it (drupal_get_path('module', 'node')). The diff module's hook_menu() and hook_menu_alter() implementation includes a different file and page callback that their file creates but doesn't explicitly specify the file path. For the same reason that the node module loses track of where the include will be, diff loses track as well. This could be solved by similarly patching diff to explicitly define 'file path' in its hook_menu and hook_menu_alter().
The Upshot
Ultimately I'm not sure what is supposed to be done about this. The implicit directory for including files when doing a hook_menu or hook_menu_alter implementation is handy and all, but I don't understand the way around it here without submitting patches for every module that explicitly state the 'file path' in each menu item.
There might be a better way to solve this that involves not altering the 'page callback' for 'node/%node/revisions'. Doing that would require the menu to be rebuilt all the time, though, so that seems ridiculous, too. Maybe the 'node/%node/moderation' is just the wrong route to go and we should be hooking the forms that 'node/%node/revisions' generate instead. I'm unsure and don't fully understand how the modules are supposed to stay out of each others' way.
Comment #2
dave reidI can't duplicate this. Drupal by default saves drupal_get_path('module', $item['module']) into the 'include_path' property which is saved into the menu router.
Comment #3
mechler commentedTo duplicate:
To further clarify: the function node_revision_overview() from the node module is not able to be found because the menu item's "file path" isn't included when we change the menu item with hook_menu_alter(). Because the .module file making the call is working from a different directory, it can't find the file implicitly. One of the suggested fixes above includes this file as required, but then when the diff module implements hook_menu_alter(), now our "file path" parameter is in place instead of their and their diff_diffs_overview() function isn't found.
Comment #4
stevectorI also cannot replicate this with your instructions mechler.
In #3 you say
However I am not finding this to be true. In this implementation of hook_menu_alter, the 'file' attribute of the 'node/%node/revisions' array is not touched. If you print out this array you should see
Array
(
[title] => Revisions
[page callback] => workbench_moderation_node_revisions_redirect
[page arguments] => Array
(
[0] => 1
)
[access callback] => _node_revision_access
[access arguments] => Array
(
[0] => 1
)
[weight] => 2
[type] => 132
[file] => node.pages.inc
[module] => node
)
That being said you and binford2k did find this error. Perhaps the safest option is to actually unset the the 'file' in hook_menu_alter and then use the include technique you layout in the first patch in #1. Thanks for reporting this and for your thorough explanation.
Comment #5
stevectorOK, so I'm able to reproduce this error, but only when I have Diff enabled. Since we have another issue specifically about Diff I am going to close this ticket and we can move forward with an integration plan over there:
#1083720: Incorporate Diff to Compare Revisions
Comment #6
technikh commentedthe other issue https://drupal.org/node/1083720 is marked fixed but I still get the error
PHP Fatal error: Call to undefined function diff_diffs_overview() in /*********/modules/workbench_moderation/workbench_moderation.module on line 230,Workbench Moderation 7.x-1.3
Diff 7.x-3.2