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
| tylerromeo(a)gmail.com
On Mon, May 13, 2013 at 2:05 PM, Chris Steipp <csteipp(a)wikimedia.org> wrote:
On Mon, May 13, 2013 at 10:26 AM, Max Semenik
<maxsem.wiki(a)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(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l