On 21/05/13 05:45, Bartosz DziewoĆski wrote:
On Mon, 20 May 2013 19:51:50 +0200, Daniel Friesen daniel@nadir-seen-fire.com wrote:
// @todo FIXME: This causes breakage in various places when we // actually expected a local URL and end up with dupe prefixes. if ( $wgRequest->getVal( 'action' ) == 'render' ) { $url = $wgServer . $url; }
That TODO has been there since 2006 (r12621), so apparently it doesn't cause that much breakage if nobody bothered to fix it.
That fixme comment is a pretty clear indication of what would break if you tried to migrate commons image descriptions from action=render to API action=parse. There's no Title.php hack or anything equivalent for the API action.
The problem here is the use of global state, necessitated by Linker interfaces. Ideally, action=render and its hypothetical API replacement would indicate that full URLs are required via ParserOptions.
Making sure every caller of a Linker static method passes an appropriate option would be tricky. Maybe a better approach would be to introduce Linker wrappers to Parser in the form of non-static methods, or to introduce a $parser->mLinker object with wrapper methods and the appropriate state.
Or, if action=render is undesirable for some reason other than the Title.php hack, you could just migrate it to the API line for line:
// @todo FIXME: This causes breakage in various places when we // actually expected a local URL and end up with dupe prefixes. if ( !empty( ApiParse::$forceAbsoluteUrls ) ) { $url = wfExpandUrl( $url, PROTO_RELATIVE ); }
At least it wouldn't be any worse than how it is now. I know there's a psychological barrier to maintaining inelegant code. But it's just psychological. Despite appearances, touching it won't make your hand dirty.
-- Tim Starling