Merge "(Bug 41352) Provide tests for edit conflicts."
[lhc/web/wiklou.git] / skins / CologneBlue.php
index 22ec0f9..84042c3 100644 (file)
@@ -55,6 +55,8 @@ class SkinCologneBlue extends SkinTemplate {
 
 class CologneBlueTemplate extends BaseTemplate {
        function execute() {
+               // Suppress warnings to prevent notices about missing indexes in $this->data
+               wfSuppressWarnings();
                $this->html( 'headelement' );
                echo $this->beforeContent();
                $this->html( 'bodytext' );
@@ -63,6 +65,7 @@ class CologneBlueTemplate extends BaseTemplate {
                $this->html( 'dataAfterContent' );
                $this->printTrail();
                echo "\n</body></html>";
+               wfRestoreWarnings();
        }
 
        /**
@@ -119,275 +122,151 @@ class CologneBlueTemplate extends BaseTemplate {
                return $this->getSkin()->getLanguage()->pipeList( $s );
        }
 
-       function bottomLinks() {
-               $sep = wfMessage( 'pipe-separator' )->escaped() . "\n";
+       /**
+        * Used in bottomLinks() to eliminate repetitive code.
+        *
+        * @param $key string Key to be passed to makeListItem()
+        * @param $navlink array Navlink suitable for processNavlinkForDocument()
+        * @param $message string Key of the message to use in place of standard text
+        *
+        * @return string
+        * @fixed
+        */
+       function processBottomLink( $key, $navlink, $message=null ) {
+               if ( !$navlink ) {
+                       // Empty navlinks might be passed.
+                       return null;
+               }
 
-               $s = '';
-               if ( $this->getSkin()->getOutput()->isArticleRelated() ) {
-                       $element[] = '<strong>' . $this->editThisPage() . '</strong>';
+               if ( $message ) {
+                       $navlink['text'] = wfMessage( $message )->escaped();
+               }
 
-                       if ( $this->getSkin()->getUser()->isLoggedIn() ) {
-                               $element[] = $this->watchThisPage();
-                       }
+               return $this->makeListItem( $key, $this->processNavlinkForDocument( $navlink ), array( 'tag' => 'span' ) );
+       }
 
-                       $element[] = $this->talkLink();
-                       $element[] = $this->historyLink();
-                       $element[] = $this->whatLinksHere();
-                       $element[] = $this->watchPageLinksLink();
-
-                       $title = $this->getSkin()->getTitle();
-
-                       if (
-                               $title->getNamespace() == NS_USER ||
-                               $title->getNamespace() == NS_USER_TALK
-                       ) {
-                               $id = User::idFromName( $title->getText() );
-                               $ip = User::isIP( $title->getText() );
-
-                               # Both anons and non-anons have contributions list
-                               if ( $id || $ip ) {
-                                       $element[] = $this->userContribsLink();
-                               }
+       // @fixed
+       function bottomLinks() {
+               $toolbox = $this->getToolbox();
+               $content_nav = $this->data['content_navigation'];
 
-                               if ( $this->getSkin()->showEmailUser( $id ) ) {
-                                       $element[] = $this->emailUserLink();
-                               }
-                       }
+               $lines = array();
 
-                       $s = implode( $element, $sep );
+               if ( $this->getSkin()->getOutput()->isArticleRelated() ) {
+                       // First row. Regular actions.
+                       $element = array();
 
-                       if ( $title->getArticleID() ) {
-                               $s .= "\n<br />";
+                       $editLinkMessage = $this->getSkin()->getTitle()->exists() ? 'editthispage' : 'create-this-page';
+                       $element[] = $this->processBottomLink( 'edit', $content_nav['views']['edit'], $editLinkMessage );
+                       $element[] = $this->processBottomLink( 'viewsource', $content_nav['views']['viewsource'], 'viewsource' );
 
-                               // Delete/protect/move links for privileged users
-                               if ( $this->getSkin()->getUser()->isAllowed( 'delete' ) ) {
-                                       $s .= $this->deleteThisPage();
-                               }
+                       $element[] = $this->processBottomLink( 'watch', $content_nav['actions']['watch'], 'watchthispage' );
+                       $element[] = $this->processBottomLink( 'unwatch', $content_nav['actions']['unwatch'], 'unwatchthispage' );
 
-                               if ( $this->getSkin()->getUser()->isAllowed( 'protect' ) ) {
-                                       $s .= $sep . $this->protectThisPage();
-                               }
+                       $element[] = $this->talkLink();
 
-                               if ( $this->getSkin()->getUser()->isAllowed( 'move' ) ) {
-                                       $s .= $sep . $this->moveThisPage();
-                               }
-                       }
+                       $element[] = $this->processBottomLink( 'history', $content_nav['views']['history'], 'history' );
+                       $element[] = $this->processBottomLink( 'info', $toolbox['info'] );
+                       $element[] = $this->processBottomLink( 'whatlinkshere', $toolbox['whatlinkshere'] );
+                       $element[] = $this->processBottomLink( 'recentchangeslinked', $toolbox['recentchangeslinked'] );
 
-                       $s .= "<br />\n" . $this->otherLanguages();
-               }
+                       $element[] = $this->processBottomLink( 'contributions', $toolbox['contributions'] );
+                       $element[] = $this->processBottomLink( 'emailuser', $toolbox['emailuser'] );
 
-               return $s;
-       }
+                       $lines[] = $this->getSkin()->getLanguage()->pipeList( array_filter( $element ) );
 
-       function editThisPage() {
-               if ( !$this->getSkin()->getOutput()->isArticleRelated() ) {
-                       $s = wfMessage( 'protectedpage' )->text();
-               } else {
-                       $title = $this->getSkin()->getTitle();
-                       if ( $title->quickUserCan( 'edit' ) && $title->exists() ) {
-                               $t = wfMessage( 'editthispage' )->text();
-                       } elseif ( $title->quickUserCan( 'create' ) && !$title->exists() ) {
-                               $t = wfMessage( 'create-this-page' )->text();
-                       } else {
-                               $t = wfMessage( 'viewsource' )->text();
-                       }
 
-                       $s = Linker::linkKnown(
-                               $title,
-                               $t,
-                               array(),
-                               $this->getSkin()->editUrlOptions()
-                       );
-               }
+                       // Second row. Privileged actions.
+                       $element = array();
 
-               return $s;
-       }
+                       $element[] = $this->processBottomLink( 'delete', $content_nav['actions']['delete'], 'deletethispage' );
+                       $element[] = $this->processBottomLink( 'undelete', $content_nav['actions']['undelete'], 'undeletethispage' );
 
-       function deleteThisPage() {
-               $diff = $this->getSkin()->getRequest()->getVal( 'diff' );
-               $title = $this->getSkin()->getTitle();
+                       $element[] = $this->processBottomLink( 'protect', $content_nav['actions']['protect'], 'protectthispage' );
+                       $element[] = $this->processBottomLink( 'unprotect', $content_nav['actions']['unprotect'], 'unprotectthispage' );
 
-               if ( $title->getArticleID() && ( !$diff ) && $this->getSkin()->getUser()->isAllowed( 'delete' ) ) {
-                       $t = wfMessage( 'deletethispage' )->text();
+                       $element[] = $this->processBottomLink( 'move', $content_nav['actions']['move'], 'movethispage' );
 
-                       $s = Linker::linkKnown(
-                               $title,
-                               $t,
-                               array(),
-                               array( 'action' => 'delete' )
-                       );
-               } else {
-                       $s = '';
-               }
+                       $lines[] = $this->getSkin()->getLanguage()->pipeList( array_filter( $element ) );
 
-               return $s;
-       }
 
-       function protectThisPage() {
-               $diff = $this->getSkin()->getRequest()->getVal( 'diff' );
-               $title = $this->getSkin()->getTitle();
-
-               if ( $title->getArticleID() && ( ! $diff ) && $this->getSkin()->getUser()->isAllowed( 'protect' ) ) {
-                       if ( $title->isProtected() ) {
-                               $text = wfMessage( 'unprotectthispage' )->text();
-                               $query = array( 'action' => 'unprotect' );
-                       } else {
-                               $text = wfMessage( 'protectthispage' )->text();
-                               $query = array( 'action' => 'protect' );
-                       }
-
-                       $s = Linker::linkKnown(
-                               $title,
-                               $text,
-                               array(),
-                               $query
-                       );
-               } else {
-                       $s = '';
+                       // Third row. Language links.
+                       $lines[] = $this->otherLanguages();
                }
 
-               return $s;
+               return implode( array_filter( $lines ), "<br />\n" ) . "<br />\n";
        }
 
-       function watchThisPage() {
-               // Cache
+       // @fixed
+       function talkLink() {
                $title = $this->getSkin()->getTitle();
 
-               if ( $this->getSkin()->getOutput()->isArticleRelated() ) {
-                       if ( $this->getSkin()->getUser()->isWatched( $title ) ) {
-                               $text = wfMessage( 'unwatchthispage' )->text();
-                               $query = array(
-                                       'action' => 'unwatch',
-                                       'token' => UnwatchAction::getUnwatchToken( $title, $this->getSkin()->getUser() ),
-                               );
-                               $id = 'mw-unwatch-link';
-                       } else {
-                               $text = wfMessage( 'watchthispage' )->text();
-                               $query = array(
-                                       'action' => 'watch',
-                                       'token' => WatchAction::getWatchToken( $title, $this->getSkin()->getUser() ),
-                               );
-                               $id = 'mw-watch-link';
-                       }
-
-                       $s = Linker::linkKnown(
-                               $title,
-                               $text,
-                               array( 'id' => $id ),
-                               $query
-                       );
-               } else {
-                       $s = wfMessage( 'notanarticle' )->text();
-               }
-
-               return $s;
-       }
-
-       function moveThisPage() {
-               if ( $this->getSkin()->getTitle()->quickUserCan( 'move' ) ) {
-                       return Linker::linkKnown(
-                               SpecialPage::getTitleFor( 'Movepage' ),
-                               wfMessage( 'movethispage' )->text(),
-                               array(),
-                               array( 'target' => $this->getSkin()->getTitle()->getPrefixedDBkey() )
-                       );
-               } else {
-                       // no message if page is protected - would be redundant
-                       return '';
+               if ( $title->getNamespace() == NS_SPECIAL ) {
+                       // No discussion links for special pages
+                       return "";
                }
-       }
-
-       function historyLink() {
-               return Linker::link(
-                       $this->getSkin()->getTitle(),
-                       wfMessage( 'history' )->escaped(),
-                       array( 'rel' => 'archives' ),
-                       array( 'action' => 'history' )
-               );
-       }
 
-       function whatLinksHere() {
-               return Linker::linkKnown(
-                       SpecialPage::getTitleFor( 'Whatlinkshere', $this->getSkin()->getTitle()->getPrefixedDBkey() ),
-                       wfMessage( 'whatlinkshere' )->escaped()
+               $companionTitle = $title->isTalkPage() ? $title->getSubjectPage() : $title->getTalkPage();
+               $companionNamespace = $companionTitle->getNamespace();
+
+               // TODO these messages appear to only be used by CologneBlue and legacy skins,
+               // kill and replace with something more sensibly named?
+               $nsToMessage = array(
+                       NS_MAIN => 'articlepage',
+                       NS_USER => 'userpage',
+                       NS_PROJECT => 'projectpage',
+                       NS_FILE => 'imagepage',
+                       NS_MEDIAWIKI => 'mediawikipage',
+                       NS_TEMPLATE => 'templatepage',
+                       NS_HELP => 'viewhelppage',
+                       NS_CATEGORY => 'categorypage',
+                       NS_FILE => 'imagepage',
                );
-       }
-
-       function userContribsLink() {
-               return Linker::linkKnown(
-                       SpecialPage::getTitleFor( 'Contributions', $this->getSkin()->getTitle()->getDBkey() ),
-                       wfMessage( 'contributions' )->escaped()
-               );
-       }
 
-       function emailUserLink() {
-               return Linker::linkKnown(
-                       SpecialPage::getTitleFor( 'Emailuser', $this->getSkin()->getTitle()->getDBkey() ),
-                       wfMessage( 'emailuser' )->escaped()
-               );
-       }
-
-       function watchPageLinksLink() {
-               if ( !$this->getSkin()->getOutput()->isArticleRelated() ) {
-                       return wfMessage( 'parentheses', wfMessage( 'notanarticle' )->text() )->escaped();
+               // Find out the message to use for link text. Use either the array above or,
+               // for non-talk pages, a generic "discuss this" message.
+               // Default is the same as for main namespace.
+               if ( isset( $nsToMessage[$companionNamespace] ) ) {
+                       $message = $nsToMessage[$companionNamespace];
                } else {
-                       return Linker::linkKnown(
-                               SpecialPage::getTitleFor( 'Recentchangeslinked', $this->getSkin()->getTitle()->getPrefixedDBkey() ),
-                               wfMessage( 'recentchangeslinked-toolbox' )->escaped()
-                       );
+                       $message = $companionTitle->isTalkPage() ? 'talkpage' : 'articlepage';
                }
-       }
 
-       function talkLink() {
-               $title = $this->getSkin()->getTitle();
-               if ( NS_SPECIAL == $title->getNamespace() ) {
-                       # No discussion links for special pages
-                       return '';
+               // Obviously this can't be reasonable and just return the key for talk namespace, only for content ones.
+               // Thus we have to mangle it in exactly the same way SkinTemplate does. (bug 40805)
+               $key = $companionTitle->getNamespaceKey( '' );
+               if ( $companionTitle->isTalkPage() ) {
+                       $key = ( $key == 'main' ? 'talk' : $key . "_talk" );
                }
 
-               $linkOptions = array();
-
-               if ( $title->isTalkPage() ) {
-                       $link = $title->getSubjectPage();
-                       switch( $link->getNamespace() ) {
-                               case NS_MAIN:
-                                       $text = wfMessage( 'articlepage' );
-                                       break;
-                               case NS_USER:
-                                       $text = wfMessage( 'userpage' );
-                                       break;
-                               case NS_PROJECT:
-                                       $text = wfMessage( 'projectpage' );
-                                       break;
-                               case NS_FILE:
-                                       $text = wfMessage( 'imagepage' );
-                                       # Make link known if image exists, even if the desc. page doesn't.
-                                       if ( wfFindFile( $link ) )
-                                               $linkOptions[] = 'known';
-                                       break;
-                               case NS_MEDIAWIKI:
-                                       $text = wfMessage( 'mediawikipage' );
-                                       break;
-                               case NS_TEMPLATE:
-                                       $text = wfMessage( 'templatepage' );
-                                       break;
-                               case NS_HELP:
-                                       $text = wfMessage( 'viewhelppage' );
-                                       break;
-                               case NS_CATEGORY:
-                                       $text = wfMessage( 'categorypage' );
-                                       break;
-                               default:
-                                       $text = wfMessage( 'articlepage' );
+               // Use the regular navigational link, but replace its text. Everything else stays unmodified.
+               $namespacesLinks = $this->data['content_navigation']['namespaces'];
+               return $this->processBottomLink( $message,  $namespacesLinks[$key], $message );
+       }
+
+       /**
+        * Takes a navigational link generated by SkinTemplate in whichever way
+        * and mangles attributes unsuitable for repeated use. In particular, this modifies the ids
+        * and removes the accesskeys. This is necessary to be able to use the same navlink twice,
+        * e.g. in sidebar and in footer.
+        *
+        * @param $navlink array Navigational link generated by SkinTemplate
+        * @param $idPrefix mixed Prefix to add to id of this navlink. If false, id is removed entirely. Default is 'cb-'.
+        */
+       function processNavlinkForDocument( $navlink, $idPrefix='cb-' ) {
+               if ( $navlink['id'] ) {
+                       $navlink['single-id'] = $navlink['id']; // to allow for tooltip generation
+                       $navlink['tooltiponly'] = true; // but no accesskeys
+
+                       // mangle or remove the id
+                       if ( $idPrefix === false ) {
+                               unset( $navlink['id'] );
+                       } else {
+                               $navlink['id'] =  $idPrefix . $navlink['id'];
                        }
-               } else {
-                       $link = $title->getTalkPage();
-                       $text = wfMessage( 'talkpage' );
                }
 
-               $s = Linker::link( $link, $text->text(), array(), array(), $linkOptions );
-
-               return $s;
+               return $navlink;
        }
 
        /**
@@ -514,13 +393,48 @@ class CologneBlueTemplate extends BaseTemplate {
        }
 
        /**
-        * @param $heading string
-        * @return string
+        * Adds CologneBlue-specific items to the sidebar: qbedit, qbpageoptions and qbmyoptions menus.
+        *
+        * @param $bar sidebar data
+        * @return array modified sidebar data
         *
         * @fixed
         */
-       function menuHead( $heading ) {
-               return "\n<h6>" . htmlspecialchars( $heading ) . "</h6>";
+       function sidebarAdditions( $bar ) {
+               // "This page" and "Edit" menus
+               // We need to do some massaging here... we reuse all of the items, except for $...['views']['view'],
+               // as $...['namespaces']['main'] and $...['namespaces']['talk'] together serve the same purpose.
+               // We also don't use $...['variants'], these are displayed in the top menu.
+               $content_navigation = $this->data['content_navigation'];
+               $qbpageoptions = array_merge(
+                       $content_navigation['namespaces'],
+                       array(
+                               'history' => $content_navigation['views']['history'],
+                               'watch' => $content_navigation['actions']['watch'],
+                               'unwatch' => $content_navigation['actions']['unwatch'],
+                       )
+               );
+               $content_navigation['actions']['watch'] = null;
+               $content_navigation['actions']['unwatch'] = null;
+               $qbedit = array_merge(
+                       array(
+                               'edit' => $content_navigation['views']['edit'],
+                               'addsection' => $content_navigation['views']['addsection'],
+                       ),
+                       $content_navigation['actions']
+               );
+
+               // Personal tools ("My pages")
+               $qbmyoptions = $this->getPersonalTools();
+               foreach ( array ( 'logout', 'createaccount', 'login', 'anonlogin' ) as $key ) {
+                       $qbmyoptions[$key] = null;
+               }
+
+               $bar['qbedit'] = $qbedit;
+               $bar['qbpageoptions'] = $qbpageoptions;
+               $bar['qbmyoptions'] = $qbmyoptions;
+
+               return $bar;
        }
 
        /**
@@ -531,117 +445,83 @@ class CologneBlueTemplate extends BaseTemplate {
         *
         * @fixed
         */
-       function quickBar(){
-               $s = "\n<div id='quickbar'>";
-
-               $sep = "<br />\n";
-
-               $plain_bar = $this->data['sidebar'];
+       function quickBar() {
+               // Massage the sidebar. We want to:
+               // * place SEARCH at the beginning
+               // * add new portlets before TOOLBOX (or at the end, if it's missing)
+               // * remove LANGUAGES (langlinks are displayed elsewhere)
+               $orig_bar = $this->data['sidebar'];
                $bar = array();
-
-               // Massage the sidebar
-               // We want to place SEARCH at the beginning and a lot of stuff before TOOLBOX (or at the end, if it's missing)
-               $additions_done = false;
-               while ( !$additions_done ) {
-                       $bar = array(); // Empty it out
-
-                       // Always display search on top
-                       $bar['SEARCH'] = true;
-
-                       foreach ( $plain_bar as $heading => $links ) {
-                               if ( $heading == 'TOOLBOX' ) {
-                                       if( $links !== NULL ) {
-                                               // If this is not a toolbox prosthetic we inserted outselves, fill it out
-                                               $plain_bar['TOOLBOX'] = $this->getToolbox();
-                                       }
-
-                                       // And insert the stuff
-
-                                       // "This page" and "Edit" menus
-                                       // We need to do some massaging here... we reuse all of the items, except for $...['views']['view'],
-                                       // as $...['namespaces']['main'] and $...['namespaces']['talk'] together serve the same purpose.
-                                       // We also don't use $...['variants'], these are displayed in the top menu.
-                                       $content_navigation = $this->data['content_navigation'];
-                                       $qbpageoptions = array_merge(
-                                               $content_navigation['namespaces'],
-                                               array(
-                                                       'history' => $content_navigation['views']['history'],
-                                                       'watch' => $content_navigation['actions']['watch'],
-                                                       'unwatch' => $content_navigation['actions']['unwatch'],
-                                               )
-                                       );
-                                       $content_navigation['actions']['watch'] = null;
-                                       $content_navigation['actions']['unwatch'] = null;
-                                       $qbedit = array_merge(
-                                               array(
-                                                       'edit' => $content_navigation['views']['edit'],
-                                                       'addsection' => $content_navigation['views']['addsection'],
-                                               ),
-                                               $content_navigation['actions']
-                                       );
-                                       $bar['qbedit'] = $qbedit;
-                                       $bar['qbpageoptions'] = $qbpageoptions;
-
-                                       // Personal tools ("My pages")
-                                       $bar['qbmyoptions'] = $this->getPersonalTools();
-                                       foreach ( array ( 'logout', 'createaccount', 'login', 'anonlogin' ) as $key ) {
-                                               $bar['qbmyoptions'][$key] = null;
-                                       }
-
-                                       $additions_done = true;
-                               }
-
-                               // Re-insert current heading, unless it's SEARCH
-                               if ( $heading != 'SEARCH' ) {
-                                       $bar[$heading] = $plain_bar[$heading];
-                               }
+               $hasToolbox = false;
+
+               // Always display search first
+               $bar['SEARCH'] = true;
+               // Copy everything except for langlinks, inserting new items before toolbox
+               foreach ( $orig_bar as $heading => $data ) {
+                       if ( $heading == 'TOOLBOX' ) {
+                               // Insert the stuff
+                               $bar = $this->sidebarAdditions( $bar );
+                               $hasToolbox = true;
                        }
 
-                       // If TOOLBOX is missing, $additions_done is still false
-                       if ( !$additions_done ) {
-                               $plain_bar['TOOLBOX'] = false;
+                       if ( $heading != 'LANGUAGES' ) {
+                               $bar[$heading] = $data;
                        }
                }
+               // If toolbox is missing, add our items at the end
+               if ( !$hasToolbox ) {
+                       $bar = $this->sidebarAdditions( $bar );
+               }
+
 
-               foreach ( $bar as $heading => $links ) {
+               // Fill out special sidebar items with content
+               $orig_bar = $bar;
+               $bar = array();
+               foreach ( $orig_bar as $heading => $data ) {
                        if ( $heading == 'SEARCH' ) {
-                               $s .= $this->menuHead( wfMessage( 'qbfind' )->text() );
-                               $s .= $this->searchForm( 'sidebar' );
-                       } elseif ( $heading == 'LANGUAGES' ) {
-                               // discard these; we display languages below page content
-                       } elseif ( $links ) {
-                               if ( is_array( $links ) ) {
-                                       // Use the navigation heading from standard sidebar as the "browse" section
-                                       if ( $heading == 'navigation' ) {
-                                               $heading = 'qbbrowse';
-                                       }
-                                       if ( $heading == 'TOOLBOX' ) {
-                                               $heading = 'toolbox';
-                                       }
+                               $bar['qbfind'] = $this->searchForm( 'sidebar' );
+                       } elseif ( $heading == 'TOOLBOX' ) {
+                               $bar['toolbox'] = $this->getToolbox();
+                       } elseif ( $heading == 'navigation' ) {
+                               // Use the navigation heading from standard sidebar as the "browse" section
+                               $bar['qbbrowse'] = $data;
+                       } else {
+                               $bar[$heading] = $data;
+                       }
+               }
 
-                                       $headingMsg = wfMessage( $heading );
-                                       $any_link = false;
-                                       $t = $this->menuHead( $headingMsg->exists() ? $headingMsg->text() : $heading );
 
-                                       foreach ( $links as $key => $link ) {
-                                               // Can be empty due to rampant sidebar massaging we're doing above
-                                               if ( $link ) {
-                                                       $any_link = true;
-                                                       $t .= $this->makeListItem( $key, $link, array( 'tag' => 'span' ) ) . $sep;
-                                               }
-                                       }
+               // Output the sidebar
+               $s = "<div id='quickbar'>\n";
 
-                                       if ( $any_link ) {
-                                               $s .= $t;
+               foreach ( $bar as $heading => $data ) {
+                       $headingMsg = wfMessage( $heading );
+                       $headingHTML = "<h6>" . ( $headingMsg->exists() ? $headingMsg->escaped() : htmlspecialchars( $heading ) ) . "</h6>";
+                       $portletId = Sanitizer::escapeId( "p-$heading" );
+                       $listHTML = "";
+
+                       if ( is_array( $data ) ) {
+                               // $data is an array of links
+                               foreach ( $data as $key => $link ) {
+                                       // Can be empty due to how the sidebar additions are done
+                                       if ( $link ) {
+                                               $listHTML .= $this->makeListItem( $key, $link );
                                        }
-                               } else {
-                                       // $links can be a HTML string
-                                       $s .= $links;
                                }
+                               if ( $listHTML ) {
+                                       $listHTML = "<ul>$listHTML</ul>";
+                               }
+                       } else {
+                               // $data is a HTML <ul>-list string
+                               $listHTML = $data;
+                       }
+
+                       if ( $listHTML ) {
+                               $s .= "<div class=\"portlet\" id=\"$portletId\">\n$headingHTML\n$listHTML\n</div>\n";
                        }
                }
 
-               $s .= $sep . "\n</div>\n";
+               $s .= "</div>\n";
                return $s;
        }