Emmanuel Engelhart wrote:
This patch provides the possibility for adding a
comment with an admin
rollback.
Neat! A couple things to work on:
The comment should be retrieved by WebRequest::getText() rather than
WebRequest::getVal(). This will perform input transcoding on the few
languages that have this extra feature (Esperanto only, currently). You
do not need to use urldecode(); this is done by PHP before we ever see
the data.
Text put into HTML output should pretty much always be escaped via
htmlspecialchars() to avoid data corruption problems and HTML/JS
injection attacks. Titles for instance may contain the & character, and
all kinds of trouble ensues if it's not escaped properly.
Title::getText() will return only the non-namespace portion, so this
doesn't work on non-article pages. You'll want to use
Title::getPrefixedText().
Use htmlspecialchars() rather than urlencode() on the user name. The URL
for the form submission will be created by the browser; we just need to
make sure that the browser gets the correct data in the first place.
Also, try to stick with tabs as Timwi's mentioned; it gets really messy
to work with files where indentation styles change constantly.
A minor UI quibble; since the comment field doesn't support line breaks,
it should probably be an <input> rather than a <textarea>. Also it might
be nice to label the comment field and make the form layout a little
more consistent with how similar features look (deletion, blocking).
-- brion vibber (brion @
pobox.com)