Page MenuHomePhabricator

Deprecate global namespace handling of the prefix keyword (reverted)
Closed, ResolvedPublic

Description

The prefix keyword allows to change the list of namespaces in the SearchContext.
If the list of searched namespaces could be fixed from the beginning this would greatly simplify the refactoring started in T185108.
I propose to change how prefix works by moving the namespace filter inside the query it builds.
While building the query if the prefix detects some incompatible settings, e.g. the namespace requested are set to NS_MAIN but the query asks for prefix:Help:Page I suggest that we emit a deprecation warning asking the user to change their query by prefixing it with all:.

UPDATE: this change caused many search forms to stop working properly. The warning message was not an appropriate solution. We will figure out another solution to move this forward.
The patch to revert this change will be deployed to production wikis on Monday May 14 at 15:00 CEST.
Deployment of this patch currently blocked by T194632.
The patch to revert has finally been deployed by @Reedy (thanks!!)

Event Timeline

dcausse created this task.Apr 30 2018, 12:01 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 429815 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/extensions/CirrusSearch@master] Prefix should not override the SearchContext namespace

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

dcausse triaged this task as Normal priority.Apr 30 2018, 2:45 PM
dcausse moved this task from needs triage to Current work on the Discovery-Search board.
dcausse moved this task from in progress to Needs review on the Discovery-Search (Current work) board.

Change 429815 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Prefix should not override the SearchContext namespace

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

TheDJ added a subscriber: TheDJ.May 11 2018, 7:38 AM

This seems to have 'broken' Extension:Inputbox, which relies a lot on the search syntax.

https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#MOS_search

Yes sadly breaking existing queries was expected and it's why we provided the warning message.
I'll take a look at InputBox to see if there's anything we can do there. We tend to move away from the prefix search syntax as it's flawed by design (it changes the namespace and is greedy), we introduced subpageof which I think cover one popular usecase of prefix.
InputBox box reports to support lucene-search but since this extension is no longer supported I suppose it's OK to break compatibility and now state that the prefix arg is only supported by CirrusSearch and change how the query is generated.

TheDJ added a comment.May 11 2018, 8:05 AM

I will note, that this currently breaks all search boxes for archives on wikipedia, which is not going to please the masses....Normally i'd say. revert. But since this is the search engine, not sure if that is really possible.

dcausse added a comment.EditedMay 11 2018, 8:14 AM

OK, would this work if :

  • revert and make prefix works the same as before
  • leave the deprecation warning stating that the behavior will change at some point and that the namespace written in the prefixkeyword must be compatible with the list of namespaces requested. I'll reinstate the deprecation once InputBox is adapted.
  • fix InputBox to generate valid queries
  • stop supporting namespace mismatch with prefix

Change 432555 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/extensions/CirrusSearch@master] Partially revert deprecation of global namespace handling in prefix

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

Change 432561 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/extensions/CirrusSearch@master] Try to set namespaces properly before suffixing the prefix keyword

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

Iniquity added a subscriber: Iniquity.EditedMay 11 2018, 10:43 AM

Why you not announced something about deploy of this feature?

Hey, I don't know why you broke prefix:namespace: search, just one note: the warning text

The namespace found in the search term prefix: is not compatible with the namespaces requested. You can fix it by prefixing your query with all:.

is a lie for all wikis except enwiki: all: search doesn't work there, and that's T165110: Let search with "all:" keyword on non-English projects, not only with its translations.

dcausse added a comment.EditedMay 11 2018, 1:34 PM

@Iniquity sorry about that, the change will be reverted soon, I underestimated the impact of the change and thought that the warning message was sufficient, this was a mistake.

@Jack_who_built_the_house true, I'll fix the issue you mention.

As for why we broke prefix:namespace: I broke it as part of a bigger refactoring to unify how we handle the search query in CirrusSearch and to address the technical debt we accumulated over time.
The prefix is problematic for 2 reasons:

  • it is greedy: this is source of confusing behaviors where users typing prefix:foo intitle:bar would expect to find pages starting with foo having bar in the title. but in fact it searches for pages starting with foo intitle:fab.
  • it bypasses the namespace selection by forcing a specific namespace (the specific thing I broke). While it may not sound important for simple queries it's a bad design in my opinion, what if another keyword allowed to change the namespace like that? What to do when we'll support OR operations between keywords? This would lead to too much ambiguity in the search syntax.
TheDJ added a comment.May 11 2018, 1:41 PM

it bypasses the namespace selection by forcing a specific namespace (the specific thing I broke). While it may not sound important for simple queries it's a bad design in my opinion, what if another keyword allowed to change the namespace like that

While i can follow the theory of this, i would like to point out, that there is nothing easier than copy pasting a title of a page into a field... which includes the namespace. It's a single unit of information from the perspective of most users.

Splitting that up into two actions, will be a usability penalty i think... maybe with input box and advanced search, that could be split up automatically with php / javascript or something, but i do think it's something that should not be underestimated.

Change 432555 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Partially revert deprecation of global namespace handling in prefix

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

Cirdan added a subscriber: Cirdan.May 13 2018, 10:29 AM

Change 432960 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/extensions/CirrusSearch@wmf/1.32.0-wmf.3] Partially revert deprecation of global namespace handling in prefix

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

dcausse renamed this task from Deprecate global namespace handling of the prefix keyword to Deprecate global namespace handling of the prefix keyword (reverted).May 14 2018, 8:26 AM
dcausse updated the task description. (Show Details)May 14 2018, 8:30 AM

Change 432960 merged by Reedy:
[mediawiki/extensions/CirrusSearch@wmf/1.32.0-wmf.3] Partially revert deprecation of global namespace handling in prefix

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

dcausse updated the task description. (Show Details)May 14 2018, 1:55 PM
dcausse added a subscriber: Reedy.

Patch deployed, prefix and associated InputBox forms should work as before.

Change 432561 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Statically reference PrefixFeature from CirrusSearch

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

debt closed this task as Resolved.Jun 1 2018, 1:52 PM
Vvjjkkii renamed this task from Deprecate global namespace handling of the prefix keyword (reverted) to bzdaaaaaaa.Jul 1 2018, 1:13 AM
Vvjjkkii reopened this task as Open.
Vvjjkkii removed dcausse as the assignee of this task.
Vvjjkkii raised the priority of this task from Normal to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: gerritbot, Aklapper.
CommunityTechBot renamed this task from bzdaaaaaaa to Deprecate global namespace handling of the prefix keyword (reverted).Jul 2 2018, 2:58 PM
CommunityTechBot closed this task as Resolved.
CommunityTechBot assigned this task to dcausse.
CommunityTechBot lowered the priority of this task from High to Normal.
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot added subscribers: gerritbot, Aklapper.