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.
I've installed the Dutch language and at /admin/config/regional/language setting page I only enabled Dutch. English is disabled.
When I'm going to the search page at /search/node/ and click advanced search, a part of the options is 'languages', and displays me both English and Dutch checkboxes.
Comment | File | Size | Author |
---|---|---|---|
#26 | 950460-26.patch | 4.82 KB | jhodgdon |
#23 | node-search-language.patch | 4.96 KB | Gábor Hojtsy |
#21 | node-search-language-test.patch | 3.29 KB | Gábor Hojtsy |
#16 | node-search-language-test.patch | 3.24 KB | Gábor Hojtsy |
#12 | node-search-language.patch | 1.7 KB | Gábor Hojtsy |
Comments
Comment #1
Jurjen de Vries CreditAttribution: Jurjen de Vries commentedI expected that the languages option is hidden when there is only 1 language enabled. And that when more languages are enabled it only shows the enabled languages.
Comment #2
jhodgdonGood catch, sounds like a bug we should fix.
Comment #3
jhodgdonComment #4
drunken monkeyAttached patch seems to fix the problem. Problem lies in node.module, though, so maybe the "Component" should be changed.
But another thing: is it just me, or does the language box break searches in general? When it shows up, I can't use the advanced search form as for any search I enter, "language:" is appended at the end and the search yields no results. If I select one or both languages, it still yields no results (but maybe only because my node are all LANGUAGE_NONE). The only way for search via the regular search form with the languages item showing up, is searching and then deleting the "language%3A" from the URL …
Also, when I check any of the checkboxes, they are blank again after submitting, this should be corrected, too (but probably in another issue).
Comment #5
jhodgdonThat's the right behavior... if your nodes are all marked LANGUAGE_NONE, then they won't show up in a language-based search, which restricts nodes to a particular language.
Having the advanced search boxes understand what search was just done is a separate issue that's been filed... let's see...
#278958: Advanced search form has usability issues
Comment #6
jhodgdonLooking at the patch, it looks like the right fix, but I think a test would be good? Things to test:
- With only English, verify no language options are shown.
- Then enable one non-English language and verify both are shown.
- Then disable English and verify that no language choices are shown.
Thoughts?
Comment #7
drunken monkeyThen the bug is that I can't NOT restrict it to a particular language – as written, even if I just submit the search form with nothing but the keywords, "language:" is added (e.g., "foo bar language:").
This seems to restrict the search to no language at all – not German, not English and also not LANGUAGE_NONE. To find all English and German results, I manually have to check both checkboxes – and there is no way whatsoever to search for content with LANGUAGE_NONE, too – at least not from the advanced search form, only with the search block.
So, to further clarify: Suppose I have three nodes – "de", "en" and "und" – with their language property the same as their title and all containing "foobar" in their body.
Regarding the test: Yes, looks like a good approach.
Comment #8
jhodgdonOK, that's a bug. I totally agree that if you don't choose a language, it shouldn't restrict you to a language, and it definitely shouldn't restrict you to no languages.
Should we file that as a separate issue to this one though?
And you are right, there should probably be an option for "Language neutral" (which is the UI text for "und"/LANGUAGE_NONE).
Comment #9
jhodgdonThis patch still needs a test, and another option for "Language neutral".
Comment #10
pwolanin CreditAttribution: pwolanin commentedThis ought to be more than normal if it breaks search when any 2nd language is enabled.
Comment #11
Gábor HojtsyI think that you cannot search for language neutral is a different bug. Seems like this issue is caused by two things though still. First, disabled languages show up in the language list, second if the language list is present, 'language:' is submitted, and therefore search happens for 'no language'.
The patch fixes the first, but the second still need attention. Once you install two more languages, the same bug will reappear. Unless you select a language, it will consider you look for "posts with no language", however, it should default to "I don't care about language'.
Comment #12
Gábor HojtsyHere is an updated patch to tackle both problems that I think belong here:
1. Disabled languages show up in search (was handled the patch already)
2. Even if you select no language, the language filter is added (this is due to array_filter() applied after the condition decides the array has elements, but array_filter() empties it out - so I moved array_filter() around)
I think that there is a missing feature that you cannot search for language neutral posts is not to be handled here.
Where should we put the test? The test would depend on node, locale and search modules working together in a certain way, right? :)
Comment #13
Gábor HojtsyLet's retitle since this is not just about enabled languages, but also about not selecting a language.
Comment #14
jhodgdonI would put the test in search.test. I think there are already some advanced search tests there, and some of them depend on other modules (such as comment), so it wouldn't be horrible to have a new test class that depends on locale.
The patch looks quite reasonable to me. A test or two would certainly be beneficial. :)
Comment #15
jhodgdonSorry, cross-posted, setting title back
Comment #16
Gábor HojtsyOk, here are elaborate tests for both aspects of the bug: disabled languages and selecting no language. Just the tests to prove first that is indeed borked.
Comment #17
Gábor HojtsyComment #18
jhodgdonI filed a separate issue for the bug about not being able to search language-neutral content:
#987210: Language-specific searches should include language neutral content
Comment #20
jhodgdonHmmm. Only one test failure in all of that? Something's probably not quite right with the tests?
Comment #21
Gábor HojtsyYup, we expect 1 failure for the two aspects of this bug respectively (in total 2 failures). The xpath check for the empty field does not really work though, since it cannot be used to check for emptyness. Instead I opted to check for the URL instead, which should stay the same if no language is selected. Now this should prove we have two fails.
Comment #23
Gábor HojtsyNow we proved it has 2 fails, here is the patch to fix them. Please review.
Comment #24
jhodgdonAssuming that the test bot result in #23 is fine (and it should be, since it's been tested manually), I think this patch is ready to go in. The tests are correctly testing the issue, and the code to fix it is clear and looks exactly right to me.
Comment #25
jhodgdonSpoke too soon. Looks like the test failed.
Comment #26
jhodgdonI ran the test locally. The problem is that if you search with no keywords, you end up on the URL 'search/node/' not 'search/node'.
So I fixed the test. I also went back and verified that it fails (locally) without the node.module hunk of the patch (the URL in that case is the incorrect "search/node/language%3A" instead of "search/node"). And I tested that the patch works manually as well (i.e. went through the reproduce steps and it's all working fine).
New patch attached. Exactly the same as the previous patch, but for the trailing / on the one test case.
I'm +1 for RTBC on this one now, assuming the test bot agrees with my local test results.
Comment #27
drunken monkeyPatch works for me, too, so RTBC as long as the test bot is happy now.
Thanks, Gábor, for fixing this!
Comment #28
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #29
Gábor HojtsySuperb, thanks Jennifer for moving this through the finish line! Yay.