Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
entity_translation_node_tab_access() overrides all other access checks for translatable nodes. Wouldn't it be better if it worked in addition to them, instead of in place of them?
Comment | File | Size | Author |
---|---|---|---|
#9 | et-tab_access-1978616-9.patch | 3.43 KB | plach |
#1 | 1978616.patch | 1 KB | douggreen |
Comments
Comment #1
douggreen CreditAttribution: douggreen commentedPatch attached
Comment #3
plachI am not sure I understand the goal of this patch: the current code checks whether core node translation or entity translation is enabled for the given node. In the first case it proxies access-control to the implementing module otherwise it performs ET-specific access control. I wouldn't expect other access callbacks to be in place in the latter case. What am I missing?
Anyway, the current patch does not look right to me, as it's mixing unrelated access callbacks.
Comment #4
douggreen CreditAttribution: douggreen commentedThe problem is that it isn't proxied when entity_translation_node('node', $node) is FALSE. translation_supported_type is not TRUE for my node because I'm only using entity_translation and this is set to ENTITY_TRANSLATION_ENABLED and not TRANSLATION_ENABLED.
Comment #5
plachLet me recap: the
node/%/translate
route is shared between (at least) ET and the core Translation module. When node translation is enabled access calbacks are proxied because we don't want to interfere with core Translation (or whatever module, such asi18n_node
, overrides its route). Instead when entity translation is enabled for the content type the only node access callback that is supposed to be executed is the one defined in the entity translation info, that isentity_translation_node_tab_access()
for nodes. What I don't understand is what access callbacks are being skipped that you think should be called.Comment #6
douggreen CreditAttribution: douggreen commented"Instead when entity translation is enabled for the content type the only node access callback that is supposed to be executed is the one defined in the entity translation info, that is entity_translation_node_tab_access() for nodes."
This prevents any custom or contrib module from having a say in the access callback. You are assuming that the only modules that care about this might be the two or three translation modules you mention.
I suspect there is a use-case you are defending that I am not seeing. But there is also a use-case here that I am either doing wrong, or you are not seeing.
In my case, each node can only be translated into certain languages. And I also modify the /translate tab using hook_preprocess_table() to only show the languages that the node supports. So I want to hide the /translate tab when the node has no available languages for translation. To do this, I've implemented hook_implements_alter() and hook_menu_alter(0 and proxied the access arguments through my own access check. But because entity translation module believes that it has the only say on this access, my module access checks never get called.
Comment #7
plachGoing the
hook_menu_alter
way here has the drawback of forcing us to skip the check for the translation method enabled for the current node. This may result in combining access checks that are meant to be used with different translation methods.I think the proper way to take control of the access checks for the
/translate
route, but only when the node is entity translation-enabled, is altering the node entity info and specifying your custom access callback there. You can read the details here:http://drupalcode.org/project/entity_translation.git/blob/refs/heads/7.x...
I'd suggest to call
entity_translation_node_tab_access()
from your access callback to ensure all the needed checks are performed.Comment #8
douggreen CreditAttribution: douggreen commentedI tried changing the entity as you suggested, using hook_entity_info_alter() and discovered that my callback was never called (on node/% pages, for example). So I reviewed the code again. And I think the problem is with the /translate tab, which is menu altered by entity_translation_node_menu_alter(). Is the problem that entity_translation_node_tab_access is used by both the entity and the menu callback, and that we want slightly different behavior for each???
Comment #9
plachOk, now I see the issue, finally :)
I think this might fix it. We should probably do the same for page callback/arguments, but I have no time right now.