r64426 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r64425‎ | r64426 | r64427 >
Date:06:48, 31 March 2010
Author:tparscal
Status:reverted (Comments)
Tags:
Comment:
Ensures empty UL elements are never included in output by testing if menus have anything in them before rendering the containing UL elements. Addresses issues noted in bug #23015
Modified paths:
  • /trunk/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/Vector.php
@@ -694,11 +694,13 @@
695695 ?>
696696 <div id="p-namespaces" class="vectorTabs<?php if ( count( $this->data['namespace_urls'] ) == 0 ) echo ' emptyPortlet'; ?>">
697697 <h5><?php $this->msg('namespaces') ?></h5>
 698+ <?php if ( count( $this->data['namespace_urls'] ) ): ?>
698699 <ul<?php $this->html('userlangattributes') ?>>
699700 <?php foreach ($this->data['namespace_urls'] as $key => $link ): ?>
700701 <li <?php echo $link['attributes'] ?>><a href="<?php echo htmlspecialchars( $link['href'] ) ?>" <?php echo $link['key'] ?>><span><?php echo htmlspecialchars( $link['text'] ) ?></span></a></li>
701702 <?php endforeach; ?>
702703 </ul>
 704+ <?php endif; ?>
703705 </div>
704706 <?php
705707 break;
@@ -707,11 +709,13 @@
708710 <div id="p-variants" class="vectorMenu<?php if ( count( $this->data['variant_urls'] ) == 0 ) echo ' emptyPortlet'; ?>">
709711 <h5><span><?php $this->msg('variants') ?></span><a href="#"></a></h5>
710712 <div class="menu">
 713+ <?php if ( count( $this->data['variant_urls'] ) ): ?>
711714 <ul<?php $this->html('userlangattributes') ?>>
712715 <?php foreach ($this->data['variant_urls'] as $key => $link ): ?>
713716 <li<?php echo $link['attributes'] ?>><a href="<?php echo htmlspecialchars( $link['href'] ) ?>" <?php echo $link['key'] ?>><?php echo htmlspecialchars( $link['text'] ) ?></a></li>
714717 <?php endforeach; ?>
715718 </ul>
 719+ <?php endif; ?>
716720 </div>
717721 </div>
718722 <?php
@@ -720,11 +724,13 @@
721725 ?>
722726 <div id="p-views" class="vectorTabs<?php if ( count( $this->data['view_urls'] ) == 0 ) echo ' emptyPortlet'; ?>">
723727 <h5><?php $this->msg('views') ?></h5>
 728+ <?php if ( count( $this->data['view_urls'] ) ): ?>
724729 <ul<?php $this->html('userlangattributes') ?>>
725730 <?php foreach ($this->data['view_urls'] as $key => $link ): ?>
726731 <li<?php echo $link['attributes'] ?>><a href="<?php echo htmlspecialchars( $link['href'] ) ?>" <?php echo $link['key'] ?>><?php echo (array_key_exists('img',$link) ? '<img src="'.$link['img'].'" alt="'.$link['text'].'" />' : '<span>'.htmlspecialchars( $link['text'] ).'</span>') ?></a></li>
727732 <?php endforeach; ?>
728733 </ul>
 734+ <?php endif; ?>
729735 </div>
730736 <?php
731737 break;
@@ -733,11 +739,13 @@
734740 <div id="p-cactions" class="vectorMenu<?php if ( count( $this->data['action_urls'] ) == 0 ) echo ' emptyPortlet'; ?>">
735741 <h5><span><?php $this->msg('actions') ?></span><a href="#"></a></h5>
736742 <div class="menu">
 743+ <?php if ( count( $this->data['action_urls'] ) ): ?>
737744 <ul<?php $this->html('userlangattributes') ?>>
738745 <?php foreach ($this->data['action_urls'] as $key => $link ): ?>
739746 <li<?php echo $link['attributes'] ?>><a href="<?php echo htmlspecialchars( $link['href'] ) ?>" <?php echo $link['key'] ?>><?php echo htmlspecialchars( $link['text'] ) ?></a></li>
740747 <?php endforeach; ?>
741748 </ul>
 749+ <?php endif; ?>
742750 </div>
743751 </div>
744752 <?php
@@ -746,11 +754,13 @@
747755 ?>
748756 <div id="p-personal" class="<?php if ( count( $this->data['personal_urls'] ) == 0 ) echo ' emptyPortlet'; ?>">
749757 <h5><?php $this->msg('personaltools') ?></h5>
 758+ <?php if ( count( $this->data['personal_urls'] ) ): ?>
750759 <ul<?php $this->html('userlangattributes') ?>>
751760 <?php foreach($this->data['personal_urls'] as $key => $item): ?>
752761 <li <?php echo $item['attributes'] ?>><a href="<?php echo htmlspecialchars($item['href']) ?>"<?php echo $item['key'] ?><?php if(!empty($item['class'])): ?> class="<?php echo htmlspecialchars($item['class']) ?>"<?php endif; ?>><?php echo htmlspecialchars($item['text']) ?></a></li>
753762 <?php endforeach; ?>
754763 </ul>
 764+ <?php endif; ?>
755765 </div>
756766 <?php
757767 break;

Follow-up revisions

RevisionCommit summaryAuthorDate
r66628Revert r64426: caused bug 23581 (CollapsibleTabs breakage) for reasons noted ...catrope21:38, 18 May 2010
r67921Revert r67799, per CR (it did the same thing as r64426, which was reverted in...catrope21:04, 12 June 2010

Comments

#Comment by Catrope (talk | contribs)   09:42, 31 March 2010

Warning: this may break scripts that add to these <ul>s and simply assume it'll always be there and never needs to be created.

#Comment by Trevor Parscal (WMF) (talk | contribs)   16:30, 31 March 2010

This is the lesser of 2 evils? This should be documented somewhere... Hmmm..

Status & tagging log