r101912 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r101911‎ | r101912 | r101913 >
Date:23:14, 3 November 2011
Author:brion
Status:reverted (Comments)
Tags:
Comment:
* (bug 31878, bug 31542) Fix XML namespace output in API; removed now-unneeded includexmlnamespace parameter.

Any existing uses of includexmlnamespace should be ignored and harmless.

If output data already includes a specific default namespace as an xmlns attribute, it is now retained -- so for example the RSD API discovery module outputs the appropriate namespace instead of the generic API one.

Partial revert of r99135 which added the includexmlnamespace parameter as a temporary requirement to get the xmlns attribute included.

Followup to r88007 which added the xmlns originally, but in the wrong order so it overrode existing output data.
Modified paths:
  • /trunk/phase3/includes/api/ApiFormatXml.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiFormatXml.php
@@ -38,7 +38,6 @@
3939 private $mRootElemName = 'api';
4040 public static $namespace = 'http://www.mediawiki.org/xml/api/';
4141 private $mDoubleQuote = false;
42 - private $mIncludeNamespace = false;
4342 private $mXslt = null;
4443
4544 public function __construct( $main, $format ) {
@@ -60,19 +59,19 @@
6160 public function execute() {
6261 $params = $this->extractRequestParams();
6362 $this->mDoubleQuote = $params['xmldoublequote'];
64 - $this->mIncludeNamespace = $params['includexmlnamespace'];
6563 $this->mXslt = $params['xslt'];
6664
6765 $this->printText( '<?xml version="1.0"?>' );
6866 if ( !is_null( $this->mXslt ) ) {
6967 $this->addXslt();
7068 }
71 - if ( $this->mIncludeNamespace ) {
72 - $data = array( 'xmlns' => self::$namespace ) + $this->getResultData();
73 - } else {
74 - $data = $this->getResultData();
75 - }
7669
 70+ // If the result data already contains an 'xmlns' namespace added
 71+ // for custom XML output types, it will override the one for the
 72+ // generic API results.
 73+ // This allows API output of other XML types like Atom, RSS, RSD.
 74+ $data = $this->getResultData() + array( 'xmlns' => self::$namespace );
 75+
7776 $this->printText(
7877 self::recXmlPrint( $this->mRootElemName,
7978 $data,
@@ -209,7 +208,6 @@
210209 return array(
211210 'xmldoublequote' => false,
212211 'xslt' => null,
213 - 'includexmlnamespace' => false,
214212 );
215213 }
216214
@@ -217,7 +215,6 @@
218216 return array(
219217 'xmldoublequote' => 'If specified, double quotes all attributes and content',
220218 'xslt' => 'If specified, adds <xslt> as stylesheet',
221 - 'includexmlnamespace' => 'If specified, adds an XML namespace'
222219 );
223220 }
224221

Follow-up revisions

RevisionCommit summaryAuthorDate
r101944Partial revert of r101912 -- restores r99315 'includexmlnamespace' parameter ...brion01:31, 4 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r88007(bug 24781) Define XML namespace for API output. Also created the referenced ...catrope16:34, 13 May 2011
r99135(bug 24781) The API will include an XML namespace if the includexmlnamespace ...btongminh20:11, 6 October 2011
r99415MFT r99135 -- partially undoes breakage from r88007...brion18:34, 10 October 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   01:28, 4 November 2011

Per list discussion we actually don't want the namespace on by default as it causes incompatibilities with namespace-aware XML parsers that have been in use by default. Heh. :)

in progress partial reverting...

#Comment by Brion VIBBER (talk | contribs)   01:31, 4 November 2011

Ok mostly reverted by r101944, but keeping the logic tweak to avoid the clobbering.

Status & tagging log