If you have an unpublished translation there will be created a language icon linking to this content on the original node, though the user has not the necessary rights to see this node.

So, please, honor the user permission in the language icons created at the bottom of the nodes and don't lead the user to "Permission denied".

btw. With the sync module it is possible to have different states with the original node and the translation node!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Freso’s picture

Title: Language Icons linking to unpublished translation » Translation links link to unpublished translation
Project: Language Icons » Drupal core
Version: 6.x-1.0 » 6.x-dev
Component: User interface » locale.module

The "Language Icons" module only adds icons to the language links; the language links themselves are created (and managed) by core's locale.module.

Pepe Roni’s picture

Version: 6.x-dev » 6.9

Thank you.

But I do not care any longer for which language dependent module is responsible for any error I find with translations, whether it is core.locale or i18n or core.sync or core.taxonomy ore core.menu or what else. Working with translations in D6 for more than 6 month I have lost the overview over the responsible modules. Translations with D6 are a hassle. If D6 (except translations) was not that easy to use and extend, I would have already switched to another CMS, even if it might have a typo in it's name ;)

Freso’s picture

Hm. Btw, is this link in the "Language switcher" block, or in the language list on the nodes themselves? If the latter, this is actually a translation.module issue. Just to confuse matters even more. :p Also, would it be possible for you to test this in a 7.x install, to see if the problem also/still resides there?

Pepe Roni’s picture

I talk about the language list on the nodes themselves, that link to the translation of the node (to another node id). The language switcher block is only used to switch the presentation language and does not change the node id if you are in the node display, thus you won't run into the described problem.

And currently I am not willing to test a 7.x install. I have to do much too much to keep a multilingual D6 site running and to teach translators how to translate this site. Did you know that you have to grant content administration rights to translators if you want them to have a fully functional workflow of translation process? But this is not part of this issue, so, please, forget it as soon as possible.

Freso’s picture

Component: locale.module » translation.module

Alright. Switching component then. ;) And I'll try and take a look at it at some point, unless someone else beats me to it.

Jody Lynn’s picture

Version: 6.9 » 7.x-dev

Bugs get fixed in HEAD and then backported.

hass’s picture

+

hass’s picture

hass’s picture

Here is a D6 patch.

hass’s picture

Status: Active » Needs review
FileSize
2.03 KB

And here the D7 for the test bot.

NOTE: This patches are not fixing the language switcher block, only the node links. I'm not sure how to fix the locale module to show only active links or links the current user have access to.

Status: Needs review » Needs work

The last submitted patch failed testing.

fletchgqc’s picture

Having the same problem (#764952: Content translation links show for unpublished nodes has been marked as a duplicate). Definitely needs fixing - I would be willing to test patches against D6. Do you want me to try the D6 patch above?

plach’s picture

Status: Needs work » Needs review
FileSize
1.44 KB

Rerolled. This could use some tests, though.

plach’s picture

I think we should clarify some aspects before going on with working on a fix. Please follow up on #778528: Define the language switcher's correct behavior.

fletchgqc’s picture

I checked out issue #778528 but it was above my head. Nevertheless I applied the D6 patch #9 manually (applying programatically didn't work for me) and it fixed the problem, it's currently working well on a production site.

GiorgosK’s picture

uploading a D6 patch that works
different approach then #9 which did not seem to work

plach’s picture

@GiorgosK:

We need to fix this in D7 before any patch can be committed to D6. Please review/test #13 if you wish to speed-up this issue.

GiorgosK’s picture

Had to apply the patch manually because it was not applied

rerolled patch but no changes in it

Reviewed and tested
behaves as expected

Status: Needs review » Needs work

The last submitted patch, 366768_translations_links_shown_for_unpublished_nodes-D7.patch, failed testing.

plach’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: +Needs tests
FileSize
1.43 KB

Rerolled.

Setting RTBC as per #18.

This still needs tests but they will have to wait for #780316: Missing node translation links when no language detection is configured to go in, as it introduces a test case for the content translation links (trying to write some tests without it would imply rewriting most of its code).

sun’s picture

Patch looks good. And actually, if this issue also exists in D6, then it could almost be understood as security issue. So let's pretend it's security hardening only.

plach’s picture

#20: translation-366768-20.patch queued for re-testing.

plach’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.41 KB

Since translation_node_get_translations() performs a node_access query probably just checking the translation status should be more performant.

Status: Needs review » Needs work

The last submitted patch, translation-366768-23.patch, failed testing.

plach’s picture

Status: Needs work » Needs review
FileSize
1.42 KB

Silly me

GiorgosK’s picture

Status: Needs review » Reviewed & tested by the community

#25 works as expected

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

plach’s picture

Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)

Great, now we need to port this to 6.x. I'll post a patch ASAP.

plach’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.99 KB

Here it is.

GiorgosK’s picture

Status: Needs review » Reviewed & tested by the community

#29 works as expected

plach’s picture

Issue tags: -Needs tests
Gábor Hojtsy’s picture

Version: 6.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs work

Hm, why not use full fledged node_access()? There might be other reasons you do not have access to that node, right?

plach’s picture

Well, $translations comes from a node_access (D7)/db_rewrite_sql (D6) query, so we are already applying node access logic or am I mistaken?

Gábor Hojtsy’s picture

Version: 7.x-dev » 6.x-dev
Status: Needs work » Fixed
FileSize
2.05 KB

Right, and this was discussed above. Ha! Ok, added one line of comment to make this more clear and committed with this change. Thanks!

// Path can only start with "node/$nid" or "node/$nid/" here.

Status: Fixed » Closed (fixed)

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