r51508 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r51507‎ | r51508 | r51509 >
Date:16:39, 5 June 2009
Author:werdna
Status:deferred
Tags:
Comment:
Fixes for LiquidThreads headers and footers:
* Remove colourisation of usernames. It's ugly, unnecessary and annoying.
* Replace hard-coded HTML with HTML generated on-the-fly with Xml class.
* Fix a possible (but presently unexploitable) XSS vector by running thread titles through the parser when displaying headings.
* Use proper Linker::Link methods, rather than hand-crafting URLs, anchors and such.
* Fix invalid XHTML, the only element valid inside a <ul> is an <li>.
* Remove pointless TODO.
Modified paths:
  • /trunk/extensions/LiquidThreads/classes/LqtView.php (modified) (history)
  • /trunk/extensions/LiquidThreads/lqt.css (modified) (history)

Diff [purge]

Index: trunk/extensions/LiquidThreads/lqt.css
@@ -412,31 +412,6 @@
413413 text-decoration: none;
414414 }
415415
416 -
417 -/* light, background color scheme:
418 -.lqt_post_color_1 a { background-color: #E0BF96 !important; }
419 -.lqt_post_color_2 a { background-color: #D2E096 !important; }
420 -.lqt_post_color_3 a { background-color: #96E0B4 !important; }
421 -.lqt_post_color_4 a { background-color: #96CCE0 !important; }
422 -.lqt_post_color_5 a { background-color: #C096E0 !important; }
423 -.lqt_post_color_6 a { background-color: #E09696 !important; }
424 -*/
425 -
426 -/* bright, forground color scheme: */
427 -.lqt_post_color_1 a { color: #cd9800 !important; }
428 -.lqt_post_color_2 a { color: #cd0098 !important; }
429 -.lqt_post_color_3 a { color: #98cd00 !important; }
430 -.lqt_post_color_4 a { color: #9800cd !important; }
431 -.lqt_post_color_5 a { color: #0098cd !important; }
432 -.lqt_post_color_6 a { color: #00cd98 !important; }
433 -
434 -.lqt_footer_sig:hover .lqt_post_color_1 a { color: #cd9800 !important; }
435 -.lqt_footer_sig:hover .lqt_post_color_2 a { color: #cd0098 !important; }
436 -.lqt_footer_sig:hover .lqt_post_color_3 a { color: #98cd00 !important; }
437 -.lqt_footer_sig:hover .lqt_post_color_4 a { color: #9800cd !important; }
438 -.lqt_footer_sig:hover .lqt_post_color_5 a { color: #0098cd !important; }
439 -.lqt_footer_sig:hover .lqt_post_color_6 a { color: #00cd98 !important; }
440 -
441416 .lqt_footer_sig:hover a { text-decoration: none; }
442417
443418 li#ca-nstab-thread {
Index: trunk/extensions/LiquidThreads/classes/LqtView.php
@@ -149,10 +149,26 @@
150150 }
151151
152152 static function permalinkUrl( $thread, $method = null, $operand = null ) {
153 - $query = $method ? "lqt_method=$method" : "";
154 - $query = $operand ? "$query&lqt_operand={$operand->id()}" : $query;
155 - return $thread->root()->getTitle()->getFullUrl( $query );
 153+ list ($title, $query) = self::permalinkData( $thread, $method, $operand );
 154+
 155+ $queryString = wfArrayToCGI( $query );
 156+
 157+ return $title->getFullUrl( $queryString );
156158 }
 159+
 160+ /** Gets an array of (title, query-parameters) for a permalink **/
 161+ static function permalinkData( $thread, $method = null, $operand = null ) {
 162+ $query = array();
 163+
 164+ if ($method) {
 165+ $query['lqt_method'] = $method;
 166+ }
 167+ if ($operand) {
 168+ $query['lqt_operand'] = $operand;
 169+ }
 170+
 171+ return array( $thread->root()->getTitle(), $query );
 172+ }
157173
158174 /* This is used for action=history so that the history tab works, which is
159175 why we break the lqt_method paradigm. */
@@ -596,39 +612,42 @@
597613 }
598614
599615 function showThreadFooter( $thread ) {
600 - global $wgLang; // TODO global.
 616+ global $wgLang, $wgUser;
 617+
 618+ $sk = $wgUser->getSkin();
 619+ $html = '';
601620
 621+ // Author signature.
602622 $author = $thread->root()->originalAuthor();
603 - $color_number = $this->selectNewUserColor( $author );
604 -
605623 $sig = $this->user->getSkin()->userLink( $author->getID(), $author->getName() ) .
606624 $this->user->getSkin()->userToolLinks( $author->getID(), $author->getName() );
 625+ $html .= Xml::tags( 'li', array( 'class' => 'lqt_author_sig' ), $sig );
607626
608 - $timestamp = $wgLang->timeanddate( $thread->created(), true );
609 -
610 - $this->output->addHTML( <<<HTML
611 -<ul class="lqt_footer">
612 -<span class="lqt_footer_sig">
613 -<li class="lqt_author_sig lqt_post_color_{$color_number}">$sig</li>
614 -HTML
615 - );
616 -
 627+ // Edited flag
617628 if ( $thread->editedness() == Threads::EDITED_BY_AUTHOR || $thread->editedness() == Threads::EDITED_BY_OTHERS ) {
618629 wfLoadExtensionMessages( 'LiquidThreads' );
619 - $editedness_url = $this->permalinkUrlWithQuery( $thread, 'action=history' );
620 - $editedness_color_number = $thread->editedness() == Threads::EDITED_BY_AUTHOR ?
621 - $color_number : ( $color_number == self::number_of_user_colors ? 1 : $color_number + 1 );
622 - $this->output->addHTML( "<li class=\"lqt_edited_notice lqt_post_color_{$editedness_color_number}\">" .
623 - '<a href="' . $editedness_url . '">' . wfMsg( 'lqt_edited_notice' ) . '</a>' . '</li>' );
 630+ list($edited_title) = $this->permalinkData( $thread );
 631+ $edited_text = wfMsgExt( 'lqt_edited_notice', 'parseinline' );
 632+ $edited_link = $sk->link( $edited_title, $edited_text,
 633+ array( 'class' => 'lqt_edited_notice'),
 634+ array( 'action' => 'history' ) );
 635+
 636+ $html .= Xml::tags( 'li', array( 'class' => 'lqt_edited_notice' ), $edited_link );
624637 }
 638+
 639+ // Timestamp
 640+ $timestamp = $wgLang->timeanddate( $thread->created(), true );
 641+ $html .= Xml::element( 'li', null, $timestamp );
625642
626 - $this->output->addHTML( "</span><li>$timestamp</li>" );
 643+ // Footer commands
 644+ $footerCommands =
 645+ $this->listItemsForCommands( $this->threadFooterCommands( $thread ) );
 646+ $html .=
 647+ Xml::tags( 'span', array( 'class' => "lqt_footer_commands" ), $footerCommands );
627648
628 - $this->output->addHTML( '<span class="lqt_footer_commands">' .
629 - $this->listItemsForCommands( $this->threadFooterCommands( $thread ) ) .
630 - '</span>' );
631 -
632 - $this->output->addHTML( '</ul>' );
 649+ $html = Xml::tags( 'ul', array( 'class' => 'lqt_footer' ), $html );
 650+
 651+ $this->output->addHTML( $html );
633652 }
634653
635654 function listItemsForCommands( $commands ) {
@@ -647,19 +666,6 @@
648667 return join( "", $result );
649668 }
650669
651 - function selectNewUserColor( $user ) {
652 - $userkey = $user->isAnon() ? "anon:" . $user->getName() : "user:" . $user->getId();
653 -
654 - if ( !array_key_exists( $userkey, $this->user_colors ) ) {
655 - $this->user_colors[$userkey] = $this->user_color_index;
656 - $this->user_color_index += 1;
657 - if ( $this->user_color_index > self::number_of_user_colors ) {
658 - $this->user_color_index = 1;
659 - }
660 - }
661 - return $this->user_colors[$userkey];
662 - }
663 -
664670 function showRootPost( $thread ) {
665671 $popts = $this->output->parserOptions();
666672 $previous_editsection = $popts->getEditSection();
@@ -707,14 +713,17 @@
708714 $commands_html = "";
709715 } else {
710716 $lis = $this->listItemsForCommands( $this->topLevelThreadCommands( $thread ) );
711 - $commands_html = "<ul class=\"lqt_threadlevel_commands\">$lis</ul>";
 717+ $commands_html = Xml::tags( 'ul',
 718+ array( 'class' => 'lqt_threadlevel_commands' ),
 719+ $lis );
712720 }
713721
714 - $html = $thread->subjectWithoutIncrement() .
715 - ' <span class="lqt_subject_increment">(' .
716 - $thread->increment() . ')</span>';
717 - $this->output->addHTML( "<h{$this->headerLevel} class=\"lqt_header\">
718 - <span class=\"mw-headline\">" . $html . "</span></h{$this->headerLevel}>$commands_html" );
 722+ $html = $this->output->parseInline( $thread->subjectWithoutIncrement() );
 723+ $html = Xml::tags( 'span', array( 'class' => 'mw-headline' ), $html );
 724+ $html = Xml::tags( 'h'.$this->headerLevel, array( 'class' => 'lqt_header' ),
 725+ $html ) . $commands_html;
 726+
 727+ $this->output->addHTML( $html );
719728 }
720729 }
721730

Status & tagging log