Page MenuHomePhabricator
Create Task
Maniphest T224353

search prefix is set to wrong object in WikimediaIncubator extension on hook SpecialSearchSetupEngine
Closed, ResolvedPublic
Actions

Description

In the hook code for hook SpecialSearchSetupEngine the display prefix for WikimediaIncubator is set on the special page SpecialSearch

	public static function onSpecialSearchSetupEngine( $search, $profile, $engine ) {
		if ( !isset( $search->prefix ) || !$search->prefix ) {
			$search->prefix = self::displayPrefix();
		}
		return true;
	}

The special page SpecialSearch $search declares a property $mPrefix.
The SearchEngine $engine declares a property $prefix

Is the prefix set on the wrong object or I am missing some magic between $mPrefix and $prefix in SpecialSearch?

I am unsure how to fix this. It looks like it is not working and dead code should be removed. Or it is better to fix and all are suppressed that things working other than before (but now right)?

<file name="includes\WikimediaIncubator.php">
  <error line="951" severity="warning" message="Reference to undeclared property \SpecialSearch-&amp;gt;prefix" source="PhanUndeclaredProperty"/>
  <error line="952" severity="warning" message="Reference to undeclared property \SpecialSearch-&amp;gt;prefix" source="PhanUndeclaredProperty"/>
</file>

Details

SubjectRepoBranchLines +/-
Use the correct object in onSpecialSearchSetupEnginemediawiki/extensions/WikimediaIncubatormaster+2 -3
Rename $search to $engine to match hook docs for SpecialSearchSetupEnginemediawiki/coremaster+15 -15
Customize query in gerrit

Related Objects
Search...

Event Timeline

Umherirrender created this task.May 25 2019, 8:49 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 25 2019, 8:49 PM
Reedy updated the task description. (Show Details)May 25 2019, 8:58 PM
Reedy subscribed.May 25 2019, 9:10 PM
		$search = $this->getSearchEngine();
		$search->setFeatureData( 'rewrite', $this->runSuggestion );
		$search->setLimitOffset( $this->limit, $this->offset );
		$search->setNamespaces( $this->namespaces );
		$search->setSort( $this->sort );
		$search->prefix = $this->mPrefix;

		Hooks::run( 'SpecialSearchSetupEngine', [ $this, $this->profile, $search ] );
		if ( !Hooks::run( 'SpecialSearchResultsPrepend', [ $this, $out, $term ] ) ) {
			# Hook requested termination
			return;
		}

I think there's been some confusion with passing the prefix between.... $search where the hook is called is a SearchEngine, but in the hook it's a SpecialPage

'SpecialSearchSetupEngine': Allows passing custom data to search engine.
$search: SpecialSearch special page object
$profile: String: current search profile
$engine: the search engine

https://github.com/wikimedia/mediawiki-extensions-WikimediaIncubator/blame/0d6276c/includes/WikimediaIncubator.php#L950-L955
https://github.com/wikimedia/mediawiki/blame/3562f0a/includes/specials/SpecialSearch.php#L317-L324

Looks like it's potentially been broken for years/since implementation...

SpecialSearch->mPrefix is protected. SearchEngine->prefix is public

I'd suggest all should be $engine->, or the params renamed

gerritbot added a comment.May 25 2019, 9:12 PM

Change 512499 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/WikimediaIncubator@master] Use the correct object in onSpecialSearchSetupEngine

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

gerritbot added a project: Patch-For-Review.May 25 2019, 9:12 PM

Change 512500 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Rename $search to $engine to match hook docs for SpecialSearchSetupEngine

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

Umherirrender added a comment.May 25 2019, 9:30 PM
In T224353#5212629, @Reedy wrote:

Looks like it's potentially been broken for years/since implementation...

Yah, that's why I ask and not directly fixed it.

The explaination with the var name in the special page and different name in the hook sounds good to me.

Reedy added a comment.May 25 2019, 9:49 PM

Also, lol https://github.com/wikimedia/mediawiki/commit/a5bbbf35c8884dae7df1bbf37fdc5f65d02c2778

gerritbot added a comment.May 25 2019, 9:50 PM

Change 512500 merged by jenkins-bot:
[mediawiki/core@master] Rename $search to $engine to match hook docs for SpecialSearchSetupEngine

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

ReleaseTaggerBot added a project: MW-1.34-notes (1.34.0-wmf.7; 2019-05-28).May 25 2019, 10:00 PM
Umherirrender added a subtask: T224777: Add phan to WikimediaIncubator extension.May 31 2019, 9:28 PM
Jdforrester-WMF closed subtask T224777: Add phan to WikimediaIncubator extension as Resolved.Jul 25 2019, 9:06 PM
Umherirrender added a comment.Jul 26 2019, 8:05 PM

The phan issue is now suppressed in the code

gerritbot added a comment.Sep 1 2019, 11:08 PM

Change 512499 merged by jenkins-bot:
[mediawiki/extensions/WikimediaIncubator@master] Use the correct object in onSpecialSearchSetupEngine

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

ReleaseTaggerBot edited projects, added MW-1.34-notes (1.34.0-wmf.21; 2019-09-03); removed MW-1.34-notes (1.34.0-wmf.7; 2019-05-28).Sep 2 2019, 12:00 AM
Reedy closed this task as Resolved.Sep 2 2019, 1:07 PM
Reedy claimed this task.
Reedy removed a project: Patch-For-Review.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL