Chris makes a good point. Also, it should be noted that the Html class does a lot more than just escape stuff. It does a whole bunch of attribute validation and standardization to make output HTML5-sanitary. While in simple cases like the one above it will not make a difference, it is probably better to maintain a uniform approach when generating HTML output.
*-- * *Tyler Romeo* Stevens Institute of Technology, Class of 2015 Major in Computer Science www.whizkidztech.com | tylerromeo@gmail.com
On Mon, May 13, 2013 at 2:05 PM, Chris Steipp csteipp@wikimedia.org wrote:
On Mon, May 13, 2013 at 10:26 AM, Max Semenik maxsem.wiki@gmail.com wrote:
Hi, I've seen recently a lot of code like this:
$html = Html::openElement( 'div', array( 'class' => 'foo' ) . Html::rawElement( 'p', array(), Html::element( 'span', array( 'id' => $somePotentiallyUnsafeId ), $somePotentiallyUnsafeText ) ) . Html::closeElement( 'div' );
IMO, cruft like this makes things harder to read and adds additional performance overhead. It can be simplified to
$html = '<div class="foo'><p>' . Html::rawElement( 'p', array(), Html::element( 'span', array( 'id' => $somePotentiallyUnsafeId ), $somePotentiallyUnsafeText ) ) . '</p></div>';
What's your opinion, guys and gals?
I'm probably a bad offender here, but you've unintentionally proved my point ;). Note that in your example, you used a single instead of a double quote after foo. Obviously, if you're using an IDE, syntax highlighting would have helped you, but my point being that when you use the classes, you're less likely to make those little mistakes that could potentially have disastrous consequences (like using single quotes around an entity and relying on htmlspecialchars for escaping, etc). And for security, I prefer for people to use whatever will cause the least amount of mistakes.
Personally also, when I'm code reviewing I don't like to see <> in the php, but that's my person preference.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l