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.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | duplicate_translations.patch | 4.14 KB | stella |
| #6 | duplicate_translations.patch | 4.15 KB | catch |
| #5 | duplicate_translations.patch | 3.25 KB | catch |
| translation-prevent-duplicate.patch | 1.45 KB | nedjo |
Comments
Comment #1
nedjoComment #2
damien tournoud commentedWe also need a check for this at submit time.
Comment #3
nedjoGood 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.
Comment #4
damien tournoud commented@nedjo: sounds good.
Comment #5
catchI 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.
Comment #6
catchDiscussed 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.
Comment #7
stella commentedI tested this and it all works great, including tests.
Cheers,
Stella
Comment #8
stella commentedA quick summary of the issue:
Issue:
Patch:
Cheers,
Stella
Comment #10
stella commentedPatch re-roll
Comment #11
stella commentedMarking back to RTBC
Comment #13
stella commentedMarking for a retest
Comment #14
stella commentedmoving back to RTBC
Comment #15
webchickOops. 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?
Comment #16
Anonymous (not verified) commentedI'm going to guess this is a won't fix for D6, since D6 commits focus on security fixes at this point.
Comment #17
Anonymous (not verified) commentedChanging issue status to reflect that it was fixed in 7.x.