Last modified: 2012-10-09 14:11:00 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T42448, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 40448 - Replace mediawiki.legacy.mwsuggest with SimpleSearch.
Replace mediawiki.legacy.mwsuggest with SimpleSearch.
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
unspecified
All All
: Normal blocker (vote)
: 1.20.0 release
Assigned To: Krinkle
:
Depends on:
Blocks: 40785
  Show dependency treegraph
 
Reported: 2012-09-22 22:56 UTC by Krinkle
Modified: 2012-10-09 14:11 UTC (History)
9 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Krinkle 2012-09-22 22:56:23 UTC
This module has been broken for a while in core.

* Uncaught TypeError: Cannot call method 'join' of null 

This is because wgSearchNamespaces is also broken (not output the same way the module is loaded, so by default the module is loaded but wgSearchNamespaces is not set).

It also seems to interfere still with Vector's SimpleSearch (in that both modules are loaded, though they don't initialize both, at least that's been fixed since)

Adding as blocker to 1.20 release. We can not release with this uncaught javascript exception on every page (that is, if the wiki has $wgEnableMWSuggest = true; which most wikis do).

We don't see the exception on Wikimedia wikis because the Vector extension overrides mwsuggest and prevents it from reaching the error.
Comment 1 Krinkle 2012-09-23 00:09:54 UTC
The following bugs can be closed when we do this:
* bug 23985
* bug 30954
Comment 2 Krinkle 2012-09-23 02:35:05 UTC
Done in Icd721011b40bb8d2c20aefa8b359a3e45413a07f.
Comment 3 Isarra 2012-09-24 19:37:13 UTC
Whatever broke, please fix it, don't remove remove a very useful feature from core in favour of a skin-specific extension, especially since many third-party wikis don't use the given skin at all, let alone the extension.
Comment 5 Krinkle 2012-09-24 20:37:57 UTC
@Jack_Phoenix and @Isarra:

The reason I prefer not to fix it is because it should be removed from core. It
is one of those features that apparently various extensions want to
re-implement.

That alone is imho enough reason to justify removing it from core, because when
it is in core, the only way to properly re-implement it is to surpress the
default implementation in an inefficient way.

That  doesn't mean the feature is useless, it means there are more ways to implement it. Yes, we should ship MediaWiki with a good implementation, and that is by default Vector's SimpleSearch (which is enabled by default as of the 1.20 bundle).

Providing a generic implementation like mediawiki.legacy.mwsuggest that is not
Vector-skin specific is perfectly justified, and I'm happy create a little
extension that does just that. People that don't use Vector could install that,
and it would do exactly what mwsuggest did in core.

But then it should still be removed from core so that people can have a choice
which implementation to install.
Comment 6 Daniel Friesen 2012-09-24 20:41:55 UTC
simplesearch happened because mwsuggest was out of date. And just as vector was written by the usability initiative to replace monobook with something more modern and usable simplesearch was created to replace mwsuggest.

So unless we have other extensions trying to provide this I do not believe that this is an edge feature we're going to have multiple extensions trying to provide.

I think simplesearch and it's improvements should be generalized and moved into core to replace mwsuggest.

I don't like the pattern of leaving core fundamental things every site should provide out of core. Especially when we go making usability improvements optional features.
Comment 7 Krinkle 2012-09-24 20:52:33 UTC
So how about moving simplesearch into core and making it not Vector-specific?

Everybody can live with that? Then I'll do that right away as a dependent commit on the removal of this old code.
Comment 8 Daniel Friesen 2012-09-24 20:53:48 UTC
(In reply to comment #7)
> So how about moving simplesearch into core and making it not Vector-specific?
> 
> Everybody can live with that? Then I'll do that right away as a dependent
> commit on the removal of this old code.

Sure...

;) Comment #6, 3rd paragraph...
Comment 9 Isarra 2012-09-24 22:20:45 UTC
(In reply to comments #6 and #7)

Yes, that would be a reasonable fix, thank you.
Comment 10 Krinkle 2012-09-25 02:41:46 UTC
Done in Icd721011b40bb8d2c20aefa8b359a3e45413a07f.
Comment 11 Krinkle 2012-09-25 02:49:49 UTC
And removed from ext.vector in Ib8ebba170fbb813a28811c91a5ec6a40a18b318d.
Comment 12 Mark A. Hershberger 2012-09-30 17:56:21 UTC
I'll make merge requests for 1.20 tarball if you don't beat me.
Comment 13 Krinkle 2012-09-30 22:33:53 UTC
I usually hold off pushing to refs/for/REL1_20 til the ref lands in master (as it may be amended until then).
Comment 14 Mark A. Hershberger 2012-10-02 14:11:12 UTC
(In reply to comment #11)
> And removed from ext.vector in Ib8ebba170fbb813a28811c91a5ec6a40a18b318d.

Could you merge that to 1.20 since it and the previous one has been merged to master?
Comment 15 Krinkle 2012-10-02 18:23:35 UTC
(In reply to comment #14)
> (In reply to comment #11)
> > And removed from ext.vector in Ib8ebba170fbb813a28811c91a5ec6a40a18b318d.
> 
> Could you merge that to 1.20 since it and the previous one has been merged to
> master?

The change in core has been backported (see Icd721011b40bb8d2c20aefa8b359a3e45413a07f).

The change in Vector extension has not been backported, but doesn't have to because the Vector has not been branched yet for 1.20, so it is automatically included in 1.20.
Comment 16 Helder 2012-10-09 14:06:09 UTC
For the record, I'm getting this with LiquidThreads:
Uncaught Error: Unknown dependency: mediawiki.legacy.mwsuggest
Comment 17 Helder 2012-10-09 14:11:00 UTC
Never mind, I was confused by this line on the source of the page:
mw.loader.load(["ext.liquidThreads","mediawiki.user","mediawiki.page.ready","mediawiki.page.watch.ajax","mediawiki.searchSuggest","ext.vector.collapsibleNav","ext.vector.collapsibleTabs","ext.vector.simpleSearch","ext.gadget.WatchlistMessages"], null, true);

It was just my copy of the vector extension which was outdated...

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links