Page MenuHomePhabricator

WVUI's TypeaheadSearch should work with a non-default `$wgScriptPath`
Closed, ResolvedPublic3 Estimated Story Points

Description

In WVUI, we use restSearchClient.ts to fetch search results for the TypeaheadSearch component. However, the url that restSearchClient constructs to fetch these results is hardcoded and assumes the mediawiki installation uses a default $wgScriptPath of /w:

const url = `//${domain}/w/rest.php/v1/search/title?${buildQueryString( params )}`; // Notice the `/w` is hardcoded

https://github.com/wikimedia/wvui/blob/9918f1027d8430ed4f155a8c61cd5f899ef25251/src/components/typeahead-search/http/restSearchClient.ts#L56

WVUI also hardcodes the script path in UrlGenerator.ts

Given that $wgScriptPath is configurable inside LocalSettings.php, we should make this configurable in WVUI as well. If we don't and the $wgScriptPath is different from the default, TypeaheadSearch won't receive any search results.

Acceptance Criteria

Developer notes

To assist the Vue.js migration team in moving this to Codex, we'll move the client into Vector as part of fixing this bug.
A client property is already supported, so we can make this change without touching WVUI.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 753569 abandoned by Jdlrobson:

[wvui@master] [Breaking] TypeaheadSearch users must define client

Reason:

Bernard is working on this. Bernard: feel free to restore/steal this code if it's useful to you :)

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

bwang removed bwang as the assignee of this task.Feb 4 2022, 8:29 PM
bwang moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.
bwang added a subscriber: bwang.

Change 759345 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Move REST search client out of WVUI into Vector

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

Seems to be blocked on this change to the jsdoc theme:
https://gerrit.wikimedia.org/r/c/jsdoc/wmf-theme/+/759750

@Volker_E can you please take a look?

Change 759345 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Move REST search client out of WVUI into Vector

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

@bwang just need to update WVUI now. Feel free to restore https://gerrit.wikimedia.org/r/c/753569. I think that can be done in parallel with QA. It may be helpful to open a subtask to track that work.

Change 753569 restored by Bernard Wang:

[wvui@master] [Breaking] TypeaheadSearch users must define client

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

Change 762530 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Add fetch tests from WVUI

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

Change 762530 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Add fetch tests from WVUI

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

bwang removed bwang as the assignee of this task.Feb 15 2022, 6:06 PM
bwang claimed this task.

Change 762940 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Add fetch tests from WVUI

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

bwang removed bwang as the assignee of this task.Feb 15 2022, 9:47 PM
bwang moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.

Change 762940 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Add fetch tests from WVUI

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

@Catrope this is a 1.38 release blocker as in 1.38 Vector 2022 is bundled with MediaWiki by default and needs to work with 3rd parties.

Please could https://gerrit.wikimedia.org/r/c/wvui/+/753569 and https://gerrit.wikimedia.org/r/c/wvui/+/764851 be prioritized. I am hoping these will be the last ever changes we need to make to WVUI and once these are done we can make the switch to Codex.

So sorry for dropping the ball on 753569! I've +2ed it, and later today I can cut a release and update MW core. However, I don't see how 753569 is technically a 1.38 release blocker, since Vector already respects $wgScriptPath. The WVUI patch seems to just be cleanup that removes the fallback default code that Vector no longer uses.

Re 764851, I think that might be more complicated than that patch makes it seem, and we're looking into that now at T303063.

Change 753569 merged by jenkins-bot:

[wvui@master] [Breaking] TypeaheadSearch users must define client

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

Jdlrobson removed a project: MW-1.38-release.

a 1.38 release blocker, since Vector already respects $wgScriptPath

Chatted with Roan and confirmed this locally. It seems https://gerrit.wikimedia.org/r/c/759345 was the only blocker and that's been resolved so untagging.

This task is now ready for sign-off. We're shipping unnecessary code in WVUI for Vector but that's not substantial enough to be a requirement of resolving this ticket.

Also affects testing on Patch Demo, downstream: https://github.com/MatmaRex/patchdemo/issues/407

Confirmed fixed on PD:

image.png (388×835 px, 35 KB)

The bottom link doesn’t work.

Steps to reproduce:

  1. Open https://patchdemo.wmflabs.org/wikis/d7c687e377/wiki/Main_Page?useskin=vector-2022
  2. Type Main in the search box.
  3. Click on Search for pages containing Main

Actual result:

  1. You get to https://patchdemo.wmflabs.org/w/index.php?title=Special%3ASearch&fulltext=1&search=Main, which doesn’t exist.

Expected result:

  1. You get to https://patchdemo.wmflabs.org/wikis/d7c687e377/w/index.php?title=Special%3ASearch&fulltext=1&search=Main.

Both the search suggestion for Main Page and the Search button work, but this bottom link doesn’t.

Change 773290 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[wvui@master] Pass formAction to footerUrl

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

Change 773292 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Pass script path to URL generator

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

Change 773290 merged by jenkins-bot:

[wvui@master] Pass formAction to footerUrl

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

Krinkle added a subscriber: Krinkle.

Setting assignee to reflect pending patch, which seems related. This task is currently blocking the release because Vector 22 currently hardcodes WMF-specific configuration which is incompatible with being bundled with MW.

Change 773292 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Pass script path to URL generator

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

Change 773311 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@REL1_38] Pass script path to URL generator

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

Change 773311 merged by jenkins-bot:

[mediawiki/skins/Vector@REL1_38] Pass script path to URL generator

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

I've backported to 1.38. Remaining work is:

  1. Cut a new release
  2. Backport new release to 1.38.

Have pinged Catrope on Slack.

Change 774947 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update WVUI v0.3.5 -> v0.4.0

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

Yes, sorry for the slow response here. The release is done, and the patch to put it in MW core is up now.

Change 775932 had a related patch set uploaded (by Jdlrobson; author: Catrope):

[mediawiki/core@REL1_38] Update WVUI v0.3.5 -> v0.4.0

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

Change 774947 merged by jenkins-bot:

[mediawiki/core@master] Update WVUI v0.3.5 -> v0.4.0

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

Change 775932 merged by jenkins-bot:

[mediawiki/core@REL1_38] Update WVUI v0.3.5 -> v0.4.0

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

Thanks @Tacsipacsi for the nudge about this one. It's in a good state now! I checked out REL1_38 and search for pages now working.