Steps to repeat:
1. Set up a site with base (English) and one other language (Swedish for example)
2. Visit admin/config/regional/translate/translate and search for
- String contains: "" (empty)
- Language: "Swedish"
- Search in: "Only untranslated strings"
- Limit search to: "Built-in interface"
Expected Results:
I expect to receive a list of strings that haven't been translated into Swedish.
Actual Results:
"No strings available."
Lengthy description:
I wanted to keep the steps to repeat fairly short, so here goes a longer description.
I'm assuming that my expectation is correct - the "Language" filter is meant to filter on target language, and in combination with the "Search in" filter we should be able to find either strings that have been translated into that language, or strings that haven't been translated into that language.
The way it currently works, the "Only untranslated strings" filter will only return strings when paired with the "English" or "All languages" settings for the Language filter. And in that case it will return "all strings that haven't been translated into any language". Which doesn't seem like a reasonable use case to me. Thus bug.
Comments
Comment #1
Carl Johan CreditAttribution: Carl Johan commentedMoving the language condition into the join solves the problem.
It ain't pretty. I'm not sure what it breaks. But it works for me?
Comment #2
Carl Johan CreditAttribution: Carl Johan commentedComment #3
Gábor HojtsySounds like indeed it would not work because you are filtering based on values in the translation table, but empty translations are currently not stored in Drupal (I believe they were stored before). Applies to Drupal 8, which means we need to fix there first based on our process.
Comment #4
Carl Johan CreditAttribution: Carl Johan commentedBut it looks like this was fixed for 8.x in #1452188: New UI for string translation.
In pretty much the same way, too. Just slightly more elegant because you can't search for english/base language strings in 8.x it seems.
The new interface looks great, btw :)
Comment #5
Carl Johan CreditAttribution: Carl Johan commentedHere's another patch that looks a bit more like the 8.x code.
Comment #6
Carl Johan CreditAttribution: Carl Johan commentedThere seems to have been a problem with the encoding..
Comment #7
Gábor HojtsyIndeed, this is fixed in Drupal 8 with the change pointed out. We did not realize this was a bug in Drupal 7 when we fixed it in Drupal 8 and it was part of a larger improvement there.
The code looks good, however, to get it committed, we'd ideally need automated tests and/or manual testing from others.
Comment #7.0
Gábor HojtsyEmphasis, wording.
Comment #8
Carl Johan CreditAttribution: Carl Johan commentedIt should be pretty easy to manually test/verify this using the steps in the description.
I was a bit unsure how to tackle adding an automated test for this, but it seems like LocaleTranslationFunctionalTest would be a good place to start.
Comment #9
Carl Johan CreditAttribution: Carl Johan commentedAll right, let's see if this test is done right.
Comment #10
Gábor HojtsyLooks good. I'm wondering why this existing test does not expose the problem for us (which you have seem to copied)?
Comment #11
Gábor HojtsyWe discussed on IRC that the existing test works differently because when looking at all languages, the language condition is not in the query, and therefore the bugos filtering of results does not happen. The patch moves the language condition into the left join to make the same behavior happen even if language is there. We agreed that the existing comment needs update on the code that I quoted. Otherwise it looks good.
Comment #12
Carl Johan CreditAttribution: Carl Johan commentedTried to explain the different intents of the tests, original comment was:
Now the two different ones are:
and
Comment #13
Carl Johan CreditAttribution: Carl Johan commentedComment #14
Gábor HojtsyIMHO this looks perfect now. As noted above it was already fixed for Drupal 8 in the same way in #1452188: New UI for string translation (because language 'all' was removed from this screen and we figured out that other languages were buggy, but did not work to backport the fix, which this issue does).
Thanks Carl for sticking to this and polishing it up!
Comment #15
David_Rothstein CreditAttribution: David_Rothstein commentedMakes sense to me... and it looks like in addition to the bug fix, a similar test already exists in Drupal 8 also (at least I think).
Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/efe6bdf
Any chance this needs to be backported to Drupal 6 too?
Comment #16
David_Rothstein CreditAttribution: David_Rothstein commentedActually, this seems important enough to mention in the release notes, and I added it to CHANGELOG.txt also: http://drupalcode.org/project/drupal.git/commit/828cd90
Comment #17
Gábor HojtsyIt does not seem to apply to Drupal 6, the condition was part of the join there, as this patch fixed for Drupal 7, and we earlier fixed for Drupal 8. http://drupalcode.org/project/drupal.git/blob/refs/heads/6.x:/includes/l...
Comment #18.0
(not verified) CreditAttribution: commentedSteps to reproduce actually only needs one language :p