r61587 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r61586‎ | r61587 | r61588 >
Date:18:59, 27 January 2010
Author:simetrical
Status:ok (Comments)
Tags:
Comment:
Use Html::element() for Xml::submitButton()

There are a bunch of other places where we could switch to Html methods
here too. See
<http://www.mediawiki.org/wiki/Special:Code/MediaWiki/61130#c5451&gt;.
Modified paths:
  • /trunk/phase3/includes/Xml.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Xml.php
——@@ -395,7 +395,7 @@
396396 * @return string HTML
397397 */
398398 public static function submitButton( $value, $attribs=array() ) {
399 - return self::element( 'input', array( 'type' => 'submit', 'value' => $value ) + $attribs );
 399+ return Html::element( 'input', array( 'type' => 'submit', 'value' => $value ) + $attribs );
400400 }
401401
402402 /**

Comments

#Comment by Platonides (talk | contribs)   21:08, 27 January 2010

Have a Xml:: function call Html doesn't seem a good idea. Plus, I don't really like that unquoted values.

I suspect this change will break several things. Probably more than having Xml::expandAttributes respect false.

#Comment by Nikerabbit (talk | contribs)   06:56, 28 January 2010

Any concrete examples of what breaks?

#Comment by Simetrical (talk | contribs)   19:18, 28 January 2010
  1. Xml::submitButton() is already outputting an HTML element, it didn't belong in Xml to begin with. We have lots of other Xml methods like it that already wrap Html methods, e.g., Xml::hidden().
  2. What unquoted values? Html will output unquoted values if possible if $wgWellFormedXml is off, which is not the default. Don't turn $wgWellFormedXml off if you don't want non-XML output.
  3. What do you think this would break? In the default configuration, the output is typically byte-for-byte identical. With $wgWellFormedXml off, it's more compact but should be treated identically by browsers (if it's not, more things but this will break).
#Comment by Platonides (talk | contribs)   21:25, 28 January 2010

1. Then why does people put it there? :( It should be moved and calls renamed.

2. Sorry, I tested the file alone, so wgWellFormedXml wasn't being taken into acocunt.

#Comment by Simetrical (talk | contribs)   21:49, 28 January 2010

The Xml class predated Html by a few years. Calls should be moved to use Html, yes. Whether we want all the special-purpose methods like submitButton() to save ourselves from having to type element( 'input', array( 'type' => 'submit' )) is a lot more debatable, though.

Status & tagging log