r67799 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r67798‎ | r67799 | r67800 >
Date:11:29, 10 June 2010
Author:platonides
Status:reverted (Comments)
Tags:
Comment:
Do not show empty <ul>
Modified paths:
  • /trunk/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/Vector.php
@@ -693,11 +693,13 @@
694694 ?>
695695 <div id="p-namespaces" class="vectorTabs<?php if ( count( $this->data['namespace_urls'] ) == 0 ) echo ' emptyPortlet'; ?>">
696696 <h5><?php $this->msg('namespaces') ?></h5>
 697+ <?php if ( count( $this->data['namespace_urls'] ) > 0 ): ?>
697698 <ul<?php $this->html('userlangattributes') ?>>
698699 <?php foreach ($this->data['namespace_urls'] as $key => $link ): ?>
699700 <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>
700701 <?php endforeach; ?>
701702 </ul>
 703+ <?php endif; ?>
702704 </div>
703705 <?php
704706 break;
@@ -714,6 +716,7 @@
715717 </h4>
716718 <?php endif; ?>
717719 <h5><span><?php $this->msg('variants') ?></span><a href="#"></a></h5>
 720+ <?php if ( count( $this->data['variant_urls'] ) > 0 ): ?>
718721 <div class="menu">
719722 <ul<?php $this->html('userlangattributes') ?>>
720723 <?php foreach ( $this->data['variant_urls'] as $key => $link ): ?>
@@ -721,6 +724,7 @@
722725 <?php endforeach; ?>
723726 </ul>
724727 </div>
 728+ <?php endif; ?>
725729 </div>
726730 <?php
727731 break;
@@ -728,11 +732,13 @@
729733 ?>
730734 <div id="p-views" class="vectorTabs<?php if ( count( $this->data['view_urls'] ) == 0 ) echo ' emptyPortlet'; ?>">
731735 <h5><?php $this->msg('views') ?></h5>
 736+ <?php if ( count( $this->data['view_urls'] ) > 0 ): ?>
732737 <ul<?php $this->html('userlangattributes') ?>>
733738 <?php foreach ( $this->data['view_urls'] as $key => $link ): ?>
734739 <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>
735740 <?php endforeach; ?>
736741 </ul>
 742+ <?php endif; ?>
737743 </div>
738744 <?php
739745 break;
@@ -740,6 +746,7 @@
741747 ?>
742748 <div id="p-cactions" class="vectorMenu<?php if ( count( $this->data['action_urls'] ) == 0 ) echo ' emptyPortlet'; ?>">
743749 <h5><span><?php $this->msg('actions') ?></span><a href="#"></a></h5>
 750+ <?php if ( count( $this->data['action_urls'] ) > 0 ): ?>
744751 <div class="menu">
745752 <ul<?php $this->html('userlangattributes') ?>>
746753 <?php foreach ($this->data['action_urls'] as $key => $link ): ?>
@@ -747,6 +754,7 @@
748755 <?php endforeach; ?>
749756 </ul>
750757 </div>
 758+ <?php endif; ?>
751759 </div>
752760 <?php
753761 break;
@@ -754,11 +762,13 @@
755763 ?>
756764 <div id="p-personal" class="<?php if ( count( $this->data['personal_urls'] ) == 0 ) echo ' emptyPortlet'; ?>">
757765 <h5><?php $this->msg('personaltools') ?></h5>
 766+ <?php if ( count( $this->data['personal_urls'] ) > 0 ): ?>
758767 <ul<?php $this->html('userlangattributes') ?>>
759768 <?php foreach($this->data['personal_urls'] as $key => $item): ?>
760769 <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>
761770 <?php endforeach; ?>
762771 </ul>
 772+ <?php endif; ?>
763773 </div>
764774 <?php
765775 break;

Follow-up revisions

RevisionCommit summaryAuthorDate
r67921Revert r67799, per CR (it did the same thing as r64426, which was reverted in...catrope21:04, 12 June 2010

Comments

#Comment by TheDJ (talk | contribs)   12:50, 10 June 2010

doesn't this break the javascript that moves the tabs between views and actions ?

#Comment by Platonides (talk | contribs)   13:57, 10 June 2010

Could be. Although I don't think it works too well. Where does it live?

#Comment by Ilmari Karonen (talk | contribs)   21:39, 10 June 2010

This will also break addPortletLink() from wikibits.js if anyone tries to use it to add new items to empty portlets.

What's the point of this change, anyway? Just shaving a few bytes off the HTML?

#Comment by Platonides (talk | contribs)   21:48, 10 June 2010

ul elements shall not be empty [1]

Yes, it is a pain, and it can be empty in html5, but with html5 = false we are serving xhtml.

I will look into addPortletLink()

#Comment by Simetrical (talk | contribs)   21:52, 10 June 2010

See r64426, r66628, and bug 23015. This was done already and reverted. If reinstituted, it needs to fix the bug that caused it to be reverted in the first place, which is more serious than standards compliance.

Status & tagging log