Page MenuHomePhabricator

Prefix search broken when using multiple namespaces and namespaces with $wgCapitalLinks = false;
Closed, ResolvedPublic

Description

It seems this has got broken again. SearchEngine::completionSearch(WithVariants) calls SearchEngine::normalizeNamespaces which unconditionally capitalizes the input based on NS_MAIN settings. Thus PrefixSearch::defaultSearchBackend it can no longer do it's magic for namespaces where titles are not capitalised.

Event Timeline

Restricted Application added a project: Discovery-Search. · View Herald TranscriptOct 29 2018, 7:03 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

@dcausse it seems you added this new code some time ago, any thoughts how to fix this?

It's very probable that I broke this feature as part of a refactoring.
normalizeNamespaces must not change the input query...
I looked at the code but it's unclear to me what could have changed this behavior.
Could you give me an example use case where this feature has been broken, I'll fix this properly and add a test so that we don't break it again. Thanks!

At http://tieteentermipankki.fi/wiki/Termipankki:Etusivu we have a modification on the skin search box that will search from multiple content namespaces. It looks like this (on the right side are the namespace names):

It fires requests like: http://tieteentermipankki.fi/w/api.php?action=opensearch&format=json&search=a&namespace=0%7C1100%7C1102%7C1104%7C1106%7C1108%7C1110%7C1112%7C1114%7C1116%7C1120%7C1122%7C1124%7C1126%7C1128%7C1130%7C1132%7C1134%7C1136%7C1138%7C1140%7C1142%7C1144%7C1146%7C1148%7C1150%7C1152%7C1154%7C1156%7C1158%7C1160%7C1162%7C1164%7C1182%7C1168%7C1170%7C1172%7C1174%7C1176%7C1178%7C1180%7C1184%7C1186%7C1188%7C1190%7C1192%7C1194%7C1196%7C1200%7C1202%7C1204%7C1206&suggest=

In our configuration, NS_MAIN has no special configuration, but the other namespaces have $wgCapitalLinkOverrides[ NS_... ] = false;.

I traced it to this line: https://github.com/wikimedia/mediawiki/blob/REL1_31/includes/search/SearchEngine.php#L446

Now I realise that the code in master is different again, and I have not tested whether this issue is fixed after the REL1_31

I did more tests and it seems it is working okay in REL1_32. Is there any simple fix that could be backported to REL1_31? Otherwise I need to wait for a stable release from REL1_32 and have it pass testing.

Oh I see, I was looking at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/444864 for a possible culprit but looks like it might fix the issue you see here. Backporting this one seems hard as it has dependent patches in CirrusSearch, I'll try to trace this down to an earlier patch to see if we can apply a quick fix.
I'll add tests for this anyways so that it won't break again.

Nikerabbit moved this task from Backlog to Needing on the User-Nikerabbit board.Nov 1 2018, 7:11 AM
EBjune assigned this task to dcausse.Nov 1 2018, 5:02 PM
EBjune triaged this task as Normal priority.

Change 471958 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/core@master] Add test for completionSearch with wgCapitalLinkOverrides

https://gerrit.wikimedia.org/r/471958

Change 471969 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/core@REL1_32] Add test for completionSearch with wgCapitalLinkOverrides

https://gerrit.wikimedia.org/r/471969

Change 471978 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/core@REL1_31] Completion search should not change the search query

https://gerrit.wikimedia.org/r/471978

Change 471979 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/core@REL1_31] Add test for completionSearch with wgCapitalLinkOverrides

https://gerrit.wikimedia.org/r/471979

Change 471958 merged by jenkins-bot:
[mediawiki/core@master] Add test for completionSearch with wgCapitalLinkOverrides

https://gerrit.wikimedia.org/r/471958

Change 471969 merged by jenkins-bot:
[mediawiki/core@REL1_32] Add test for completionSearch with wgCapitalLinkOverrides

https://gerrit.wikimedia.org/r/471969

Change 471978 merged by jenkins-bot:
[mediawiki/core@REL1_31] Completion search should not change the search query

https://gerrit.wikimedia.org/r/471978

Change 471979 merged by jenkins-bot:
[mediawiki/core@REL1_31] Add test for completionSearch with wgCapitalLinkOverrides

https://gerrit.wikimedia.org/r/471979

debt closed this task as Resolved.Nov 29 2018, 7:52 PM