If a user returns to a page where a translation was previously created, s/he gets the same form and can create a duplicate translation.

Steps to recreate:

* Designate the page content type as translatable
* Enable two languages (en, fr)
* Create a page in en
* Translate to fr, noting the page, e.g., node/add/page?translation=1&language=fr
* After saving, return to the same page. Fill out the form again and submit.

Result:

* Two different translations in French, both with the same tnid.

Attached patch prevents this error by testing for existing translations in the requested language.

CommentFileSizeAuthor
#10 duplicate_translations.patch4.14 KBstella
#6 duplicate_translations.patch4.15 KBcatch
#5 duplicate_translations.patch3.25 KBcatch
translation-prevent-duplicate.patch1.45 KBnedjo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented
Issue tags: +i18n sprint
Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Needs review » Needs work

We also need a check for this at submit time.

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented

Good point.

I guess this would be a translation_nodeapi_validate() function:

* Test for node type is translatable

* Determine the applicable tnid by checking node->tnid or node->translation_source->tnid

* get translations (translation_node_get_translations($tnid), set form error on 'language' field if the language value is already in the translations array.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

@nedjo: sounds good.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs work » Needs review
FileSize
duplicate_translations.patch3.25 KB

I added translation_nodeapi_validate() but then discovered that hook_nodeapi_prepare() actually fires even when using the back button, and since it was wiping out the node->language etc. before we got to submit, we end up with the same drupal_set_message() and no duplicate on submit anyway.

So, this patch keeps the _prepare(), doesn't add a validate, and adds tests for both visiting the node/add page again manually, and also a resubmission of the same form (which is equivalent to using the back button).

In both cases, you can create the node you're posting, but it'll simply not have a tnid - it'll be new content in it's own right. I'm not convinced this is ideal from a usability standpoint (although you're already at this page incorrectly anyway if you get there) - but it does fix the bug.

This could probably do with some manual testing by someone to confirm the same behaviour I'm getting - i.e. that we really don't need a _nodeapi_validate(). I think if we were to remove the early return; in _prepare() then we'd actually need the _validate() as well, which'd allow for form_set_error() to prevent posting at all - but that's adding extra code to do much the same thing, so if it's not needed, I don't think we should go out of our way to add it in.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
FileSize
duplicate_translations.patch4.15 KB

Discussed this with nedjo in irc, and there's situtations where you might get the _validate() being called in contrib even if it doesn't happen in core. So I've added that back in.

stella’s picture

stella CreditAttribution: stella commented

I tested this and it all works great, including tests.

Cheers,
Stella

stella’s picture

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

A quick summary of the issue:

Issue:

  • You can create multiple translations of a node, in the same language, by visiting the 'add translation' url directly, e.g. node/add/page?translation=1&language=fr

Patch:

  • Ensures a translation for that node and language doesn't already exist before displaying the 'add node' form, and before it is saved to the database.
  • An error message is displayed if the translation is already present.
  • Includes a test which ensures duplicate translations can not be created.

Cheers,
Stella

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

stella’s picture

stella CreditAttribution: stella commented
Status: Needs work » Needs review
FileSize
duplicate_translations.patch4.14 KB

Patch re-roll

stella’s picture

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

Marking back to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

stella’s picture

stella CreditAttribution: stella commented
Status: Needs work » Needs review

Marking for a retest

stella’s picture

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

moving back to RTBC

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Version: 7.x-dev » 6.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Oops. Sorry! I thought I had committed this already!

Committed to HEAD. :)

Marking down to 6.x, as I believe this is an issue there as well, yes?

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Patch (to be ported) » Closed (won't fix)

I'm going to guess this is a won't fix for D6, since D6 commits focus on security fixes at this point.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Version: 6.x-dev » 7.x-dev
Status: Closed (won't fix) » Closed (fixed)

Changing issue status to reflect that it was fixed in 7.x.