Last modified: 2011-11-10 01:44:13 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 T29730, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 27730 - Move mw.uri module into the core
Move mw.uri module into the core
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
1.18.x
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch, patch-reviewed
Depends on: 27876
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-26 00:31 UTC by Helder
Modified: 2011-11-10 01:44 UTC (History)
5 users (show)

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


Attachments
Addition of '*' to get all url parameters (1.63 KB, patch)
2011-02-26 01:22 UTC, Helder
Details

Description Helder 2011-02-26 00:31:16 UTC
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]]
)
Comment 1 Helder 2011-02-26 01:22:59 UTC
Created attachment 8213 [details]
Addition of '*' to get all url parameters

This patch (based on [[Wikipedia:WikiProject User scripts/Scripts/Revert tools]]) adds the requested feature.
Comment 2 Michael Dale 2011-02-26 02:04:29 UTC
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 )
Comment 3 Michael Dale 2011-03-08 04:37:06 UTC
(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?
Comment 4 Krinkle 2011-03-08 08:18:20 UTC
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>
Comment 5 Michael Dale 2011-03-08 15:35:21 UTC
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.
Comment 6 Neil Kandalgaonkar 2011-03-08 18:14:26 UTC
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
Comment 7 Krinkle 2011-03-08 19:56:06 UTC
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
Comment 8 p858snake 2011-04-30 00:08:54 UTC
*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*
Comment 9 Sumana Harihareswara 2011-11-09 23:35:34 UTC
+reviewed since it looks like the patch has been reviewed, and possibly superseded by mw.uri.  Thank you for the patch, mybugs!
Comment 10 Krinkle 2011-11-10 01:44:13 UTC
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"

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


Navigation
Links