r56778 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r56777‎ | r56778 | r56779 >
Date:17:41, 22 September 2009
Author:simetrical
Status:ok (Comments)
Tags:
Comment:
Fix "Invalid argument for foreach()" in Html

Reported by Nikerabbit on IRC to happen on Preferences, although I
couldn't reproduce immediately. The change should be helpful for this
kind of thing anyway.
Modified paths:
  • /trunk/phase3/includes/Html.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Html.php
——@@ -107,6 +107,7 @@
108108 */
109109 public static function rawElement( $element, $attribs = array(), $contents = '' ) {
110110 global $wgHtml5, $wgWellFormedXml;
 111+ $attribs = (array)$attribs;
111112 # This is not required in HTML 5, but let's do it anyway, for
112113 # consistency and better compression.
113114 $element = strtolower( $element );

Follow-up revisions

RevisionCommit summaryAuthorDate
r56821Improve $attribs documentation in Html...simetrical15:16, 23 September 2009

Comments

#Comment by Nikerabbit (talk | contribs)   19:41, 22 September 2009

Does the cast actually do anything useful except if null is given? For false and string values it seems just to hide bugs.

#Comment by Simetrical (talk | contribs)   19:52, 22 September 2009

false should be cast to empty array, which seems reasonable. A string will be cast to the array containing that string, which could be theoretically useful for boolean attributes (although it's unlikely that the only attribute an element has would be boolean). I doubt it's worth adding extra lines to make a distinction.

#Comment by Nikerabbit (talk | contribs)   07:10, 23 September 2009

False and true work like a string: 

> var_dump( (array)false );
array(1) {
  [0]=>
  bool(false)
}

# For boolean attributes, support array( 'foo' ) instead of
# requiring array( 'foo' => 'meaningless' ).

Should this be mentioned in higher level documentation than in inline function comments?

#Comment by Simetrical (talk | contribs)   15:20, 23 September 2009

Sigh, typical of PHP to handle false and null differently here for no apparent reason. The effect of an array with sole value false will be to emit nothing anyway, though, which IMO is expected. (It will treat it as an attribute-value pair with key "0" and value false, where a value of false means to suppress the attribute.) More error-checking on the sanity of attribute keys/values would be reasonable to add.

I've improved comments on handling of boolean attributes in r56821.

Status & tagging log