Page MenuHomePhabricator

Move mw.uri module into the core
Closed, ResolvedPublic

Description

With the current implementation of mw.util.getParamValue, if a script needs to use N url parameters it will need to do N calls to the function, and this means there will be created N RegExp objects, one for each requested parameter.

It would be better to have a way of requesting all url paramenters at once. The function could return an object, giving us something similar to
var p = {
'parameter' : 'value',
'foo' : 'bar',
'etc' : '...'
}

which could be used as in

if ( p['parameter'] === 'value' ) {
Do something
} else if ( p['foo'] === 'bar' ) {
Do something else
}

One way to implement the feature would be to split the url after the first "?" and then break it in parts separated by "&", getting an array of "param=value" pair.
(PS: This is what was made e.g. in the following script:
[[w:pt:Wikipedia:Software/Scripts/Reversão_e_avisos.js]]
)


Version: 1.18.x
Severity: enhancement

Details

Reference
bz27730

Related Objects

StatusSubtypeAssignedTask
ResolvedNone
ResolvedNone

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:24 PM
bzimport set Reference to bz27730.
bzimport added a subscriber: Unknown Object (MLST).

Created attachment 8213
Addition of '*' to get all url parameters

This patch (based on [[Wikipedia:WikiProject User scripts/Scripts/Revert tools]]) adds the requested feature.

Attached:

mdale wrote:

I would vote for getParamValue to be deprecated and we copy in the parseuri library:

http://blog.stevenlevithan.com/archives/parseuri

Its only a few more lines and covers all our foreseeable URL extraction needs. ( I make heavy use of it in mwEmbed modules / extensions / gadgets )

mdale wrote:

(In reply to comment #2)

I would vote for getParamValue to be deprecated and we copy in the parseuri
library:

http://blog.stevenlevithan.com/archives/parseuri

Its only a few more lines and covers all our foreseeable URL extraction needs.
( I make heavy use of it in mwEmbed modules / extensions / gadgets )

looks like Neil has been working on an enhanced version of parseURI called mw.Uri ( in the upload wizard resources ) maybe take a look at that?

In one of the development ideas for mw.util was a getAllParams.

One of weak spots it fixed was duplicate values and problems with the #-tag.
I'll commit this tonight, not sure why it wasn't added until now.

Using string split does not seem a solid approach to me though. Consider the following:

/path.to?key=value&key=bettervalue&foo=bar#!testingfoo&some&key=1

The values need to be the same as for mw.util.getParamValue:

<pre>
parseUrlParams: function( url ) {

		url = url ? url : document.location.href;
		var match = url.match(/\?[^#]*/);
		if (match === null) {
			return null;
		}
		var query = match[0];
		var ret = {};
		var pattern = /[&?]([^&=]*)=?([^&]*)/g;
		match = pattern.exec(query);
		for (; match !== null; match = pattern.exec(query)) {
			var key = decodeURIComponent(match[1]);
			var value = decodeURIComponent(match[2]);
			ret[key] = value;
		}
		return ret;

}
</pre>

mdale wrote:

I would prefer if we just have a general Uri handler that can do everything from parsing paths, hosts and parameters let you modify any of thous values and then get the reconstructed uri. mw.Uri is a step in the right direction... custom functions for bits and pieces of Uri handling in the utilities class is a ~not such a good~ direction.

neilk wrote:

Yeah, one of the reasons I wrote mw.Uri.js was to avoid death by a million URI manipulation functions. It parses a URI very nicely and then you can do almost anything to it.

However, one thing it does not do is repeated query args, e.g. key=value&key=anotherValue -- just added #27942 for that

Changing summary again to move that module into the core.

Since none of the more advanced functions is used in core (yet) I suggest we keep that module optional and loaded whenever needed (ie. an extension could define it as a dependancy in which case it'll will be loaded on every page the extensions is on in the same http request.)

(In reply to comment #5)

I would prefer if we just have a general Uri handler that can do everything
from parsing paths, hosts and parameters let you modify any of thous values and
then get the reconstructed uri. mw.Uri is a step in the right direction...
custom functions for bits and pieces of Uri handling in the utilities class is
a ~not such a good~ direction.

I agree. Now that I know mw.uri exists I think it's great to have it into core as it's own dedicated module. There's lots of more cool stuff in there [1].

It does possibly require a bit of updating when moved in core though, as it was written before the mw library was created in core:

  • making sure it doesn't rely on anything from the UploadWizard,
  • Small things like mw.isEmpty > $.isEmpty
  • No need to map window.mediaWiki to mw (mw and jQuery and globals in core, $ should still be wrapped though (for now))
  • lowerCamelCase instead of UpperCamels

Also a few things that could perhaps be made more effecient/correct.

  • Functions and variables that are called 'private' but are in fact not private.
  • Static things that aren't static (anything that is common to all instances and should never change can be made private/static by puttig it in a local variable that way it's not re-created on every instance (faster) and is also private for real (like the comments say now) ).

Anyway, awesome script! extend() and clone() are very neat as well.

As getting parameter values wikiEncoding (preserving slash and colons etc.) is by far the most common use of url functions (atleast in core js and in all gadgets/tools I've seen) I suggest we do keep that those stand-alone in mw.util without dependancies.

Krinkle

[1] http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/UploadWizard/resources/mw.Uri.js?view=markup

*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*

sumanah wrote:

+reviewed since it looks like the patch has been reviewed, and possibly superseded by mw.uri. Thank you for the patch, mybugs!

The more complete mw.Uri object constructor was moved into core by neilk in r93781. I'm not sure we should make mw.util any more complex / duplicate functionality.

Marking as fixed.

  • Example:

// load module 'mediawiki.Uri'

var url = new mw.Uri( 'http://path.to/?key=value&key=bettervalue&foo=bar#!testingfoo&some&key=1' );

url.query.foo; // "bar"