Page MenuHomePhabricator

Syntax to customise location of search box within navigation bar
Closed, ResolvedPublic

Description

Author: gatoatigrado

Description:
A consensus has been reached to redesign the sidebar. The programming effort is
at
http://en.wikipedia.org/wiki/Wikipedia_talk:Village_pump_%28proposals%29/Sidebar_redesign/programming.
Coding is complete, and waiting for review and integration.

Thank you,
Nicholas Tung, wikipedia user gatoatigrado
gatoatigrado@myrealbox.com, ntung.com


Version: unspecified
Severity: enhancement

Details

Reference
bz7493

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 9:27 PM
bzimport set Reference to bz7493.
bzimport added a subscriber: Unknown Object (MLST).

Linked page neither explains what it does nor includes any code.
What on earth is this?

gatoatigrado wrote:

Follow the User:Gatoatigrado/sidebarhack link. The project page is at
http://en.wikipedia.org/wiki/Wikipedia:Village_pump_%28proposals%29/Sidebar_redesign.

gatoatigrado wrote:

Brion I sent you a message on your wikipedia talk page explaining all of
this...I guess you didn't read it.

gatoatigrado wrote:

I think you have to replace the logo manually. I don't know about the alpha
fixes for ie6 though, so you might want to wait. Apparently someone made a nice
javascript to fix all of them when the page loads.

What is this about nested if statements and parser functions?
How does that relate to adding a few links to a sidebar?
What does that have to do with a logo?

gatoatigrado wrote:

"What is this about nested if statements and parser functions?" - this was for a
different approach. Sometime, whether for only my site or not, I want to have a
custom sidebar based on lambda functions. This way, it could be efficiently
cached and dynamic content could be placed in the navigation links, etc.; where
ever it should be, not in one box because it is the only dynamic one. But this
is a far-off proposal.

The searchbox needed to be moved up, and the horizontal line needed to be added
to the toolbox (along with reorganization). Moving the searchbox necessitates a
change to MediaWiki:Sidebar which is not backwards compatible, so
MediaWiki:Monobooksidebar needs to be used. Things get a bit complicated then,
particularly with the caching.

The logo, as you can read on the project page, doesn't render the alphas
correctly on the sides of the Wikipedia logo.

Please remove the bits that aren't related to the proposal, and if there's some
code actually written please attach it to this bug where it can be found and read.

gatoatigrado wrote:

In Skin.php, "if($heading == "#searchbox") { $result[$heading] =
'special:searchbox'; }" was probably the most significant addition, but the
caching needed to be optimized. If a cached version of MediaWiki:Monobooksidebar
exists, it is returned, otherwise, if there is no MediaWiki:Monobooksidebar,
MediaWiki:Sidebar is tried (cached, then parsed). If there is a
MediaWiki:Monobooksidebar, no cache it will parse it.

Unfortunately Linux's diff messes up; it doesn't show the logical additions,
instead removing large blocks of code and inserting them again.

In Monobook.php, the search output was moved into a function. It will be
returned only once. This will occur either when the search box header occurs, or
when all of the static sidebar has been parsed. The toolbox code was also
cleaned up, and the horizontal line was added. You can see the difference by
following the link in the "output" section. I will attach diffs anyway, give me
a second.

gatoatigrado wrote:

diff for skin template

attachment skin_diff.diff ignored as obsolete

gatoatigrado wrote:

diff for skin

attachment skin_diff.diff ignored as obsolete

gatoatigrado wrote:

diff for monobook

attachment monobook.diff ignored as obsolete

gatoatigrado wrote:

This is MediaWiki 1.7.1. You need to be an admin and create
MediaWiki:Monobooksidebar for the sidebar to make a difference (copy the code
from the User:Gatoatigrado/sidebarhack page).

gatoatigrado wrote:

"Unfortunately Linux's diff messes up; it doesn't show the logical additions,
instead removing large blocks of code and inserting them again." - oh yeah, I
renamed $bar to $result, feel free to rename it back, but I have no idea what
$bar meant. $result is much easier to understand. This is probably why.

I am trying all of this code out (with and without caching) so it should work
for you.

  1. Always use unified diffs, which are more readable and can be applied to

modified files more reliably. Simply issue 'svn diff' for this.

  1. Make patches against current trunk in SVN, not the 3-month-old previous

release. Many things have changed, and old patches may not longer apply.

  1. Put all patches that go together in one file and one attachment.
  1. Mark the attachments as patches in bugzilla so they can be read online.

gatoatigrado wrote:

svn diff what? please just tell me the exact arguments to save me time. thanks.
can you give me your repository url?
Will svn diff put all of the patches in one file?

I have to go now; I'll get to it later.

"svn diff" is the entire command.
Redirect its output to a file as you would any other command.

Please see: http://www.mediawiki.org/wiki/Subversion

gatoatigrado wrote:

wow that was a lot easier than I thought it would be. Worked fine for me. Thanks.

gatoatigrado wrote:

patch for new sidebar functions

attachment sidebar_diff_2006-10-04.diff ignored as obsolete

gatoatigrado wrote:

Comment on attachment 2441
diff for skin template

Index: skins/MonoBook.php

  • skins/MonoBook.php (revision 16806)

+++ skins/MonoBook.php (working copy)
@@ -38,6 +38,26 @@

  • @subpackage Skins */ class MonoBookTemplate extends QuickTemplate {

+ var $wrote_searchbox;
+ // write the searchbox
+ function write_searchbox() {
+ if(!isset($this->wrote_searchbox)) { $this->wrote_searchbox =
true; }
+ else { return; } ?>
+ <div id="p-search" class="portlet">
+ <h5><label for="searchInput"><?php $this->msg('search')
?></label></h5>
+ <div id="searchBody" class="pBody">
+ <form action="<?php $this->text('searchaction') ?>"
id="searchform"><div>
+ <input id="searchInput" name="search"
type="text" <?php
+ if($this->haveMsg('accesskey-search'))
{
+ ?>accesskey="<?php
$this->msg('accesskey-search') ?>"<?php }
+ if( isset( $this->data['search'] ) ) {
+ ?> value="<?php
$this->text('search') ?>"<?php } ?> />
+ <input type='submit' name="go"
class="searchButton" id="searchGoButton" value="<?php $this->msg('go') ?>"
/>&nbsp;
+ <input type='submit' name="fulltext"
class="searchButton" value="<?php $this->msg('search') ?>" />
+ </div></form>
+ </div>
+ </div>
+<?php }
/**

  • Template filter callback for MonoBook skin.
  • Takes an associative array of data set from a SkinTemplate-based

@@ -143,7 +163,13 @@

			?>title="<?php $this->msg('mainpage') ?>"></a>

</div>
<script type="<?php $this->text('jsmimetype') ?>"> if (window.isMSIE55)
fixalpha(); </script>

  • <?php foreach ($this->data['sidebar'] as $bar => $cont) { ?>

+ <?php
+ foreach ($this->data['sidebar'] as $bar => $cont) {
+ if($bar == '#searchbox' && $cont ===
'special:searchbox')
+ {
+ $this->write_searchbox();
+ continue;
+ } ?>
<div class='portlet' id='p-<?php echo htmlspecialchars($bar) ?>'>

		<h5><?php $out = wfMsg( $bar ); if (wfEmptyMsg($bar, $out))

echo $bar; else echo $out; ?></h5>

		<div class='pBody'>

@@ -156,69 +182,32 @@

			</ul>
		</div>

</div>

  • <?php } ?>
  • <div id="p-search" class="portlet">
  • <h5><label for="searchInput"><?php $this->msg('search')

?></label></h5>

  • <div id="searchBody" class="pBody">
  • <form action="<?php $this->text('searchaction') ?>"

id="searchform"><div>

  • <input id="searchInput" name="search"

type="text" <?php

  • if($this->haveMsg('accesskey-search'))

{

  • ?>accesskey="<?php

$this->msg('accesskey-search') ?>"<?php }

  • if( isset( $this->data['search'] ) ) {
  • ?> value="<?php

$this->text('search') ?>"<?php } ?> />

  • <input type='submit' name="go"

class="searchButton" id="searchGoButton" value="<?php
$this->msg('searcharticle') ?>" />&nbsp;

  • <input type='submit' name="fulltext"

class="searchButton" value="<?php $this->msg('searchbutton') ?>" />

  • </div></form>
  • </div>
  • </div>

+ <?php } $this->write_searchbox(); ?>
<div class="portlet" id="p-tb">

		<h5><?php $this->msg('toolbox') ?></h5>
		<div class="pBody">
			<ul>

<?php

  • if($this->data['notspecialpage']) { ?>
  • <li id="t-whatlinkshere"><a href="<?php
  • echo

htmlspecialchars($this->data['nav_urls']['whatlinkshere']['href'])

  • ?>"><?php $this->msg('whatlinkshere')

?></a></li>
-<?php

  • if( $this->data['nav_urls']['recentchangeslinked'] ) {

?>

  • <li id="t-recentchangeslinked"><a href="<?php
  • echo

htmlspecialchars($this->data['nav_urls']['recentchangeslinked']['href'])

  • ?>"><?php $this->msg('recentchangeslinked')

?></a></li>
-<?php }

  • }
  • if(isset($this->data['nav_urls']['trackbacklink'])) { ?>
  • <li id="t-trackbacklink"><a href="<?php
  • echo

htmlspecialchars($this->data['nav_urls']['trackbacklink']['href'])

  • ?>"><?php $this->msg('trackbacklink')

?></a></li>
-<?php }

  • if($this->data['feeds']) { ?>

+ $text_before_line = false;
+ if($this->data['feeds']) { $text_before_line = true; ?>

			<li id="feedlinks"><?php foreach($this->data['feeds']

as $key => $feed) {

					?><span id="feed-<?php echo

htmlspecialchars($key) ?>"><a href="<?php

					echo htmlspecialchars($feed['href'])

?>"><?php echo htmlspecialchars($feed['text'])?></a>&nbsp;</span>

					<?php } ?></li><?php
		}
  • foreach( array('contributions', 'blockip', 'emailuser',

'upload', 'specialpages') as $special ) {

  • if($this->data['nav_urls'][$special]) {
  • ?><li id="t-<?php echo $special ?>"><a

href="<?php echo htmlspecialchars($this->data['nav_urls'][$special]['href'])

  • ?>"><?php $this->msg($special) ?></a></li>

+ foreach( array('permalink', 'print', 'recentchangeslinked',
'whatlinkshere',
+ 'trackbacklink', 'contributions', 'blockip',
'emailuser', '-', 'recentchanges', 'specialpages', 'upload') as $special ) {
+ if($special == '-' && $text_before_line) {
echo("\t\t\t</ul>\n\t\t\t\t<hr />\n\t\t\t<ul>\n"); }
+ else if($this->data['nav_urls'][$special]) {
+ $text_before_line = true;
+ $link =
$this->data['nav_urls'][$special]['href'];
+ ?>
+ <li id="t-<?php if($link == '') { echo "is"; }
echo $special ?>"><?php if($link !== '') { ?><a href="<?php echo
htmlspecialchars($link)
+ ?>"><?php } ?><?php if($special == 'print') {
$special = 'printableversion'; } $this->msg($special) ?><?php if($link !== '')
{ ?></a><?php } ?></li>
<?php }

		}
  • if(!empty($this->data['nav_urls']['print']['href'])) { ?>
  • <li id="t-print"><a href="<?php echo

htmlspecialchars($this->data['nav_urls']['print']['href'])

  • ?>"><?php $this->msg('printableversion')

?></a></li><?php

  • } -
  • if(!empty($this->data['nav_urls']['permalink']['href'])) { ?>
  • <li id="t-permalink"><a href="<?php echo

htmlspecialchars($this->data['nav_urls']['permalink']['href'])

  • ?>"><?php $this->msg('permalink')

?></a></li><?php

  • } elseif ($this->data['nav_urls']['permalink']['href'] === '')

{ ?>

  • <li id="t-ispermalink"><?php

$this->msg('permalink') ?></li><?php

  • } -
		wfRunHooks( 'MonoBookTemplateToolboxEnd', array( &$this ) );

?>

			</ul>

Index: includes/SkinTemplate.php

  • includes/SkinTemplate.php (revision 16806)

+++ includes/SkinTemplate.php (working copy)
@@ -812,6 +812,7 @@

				$nav_urls['upload'] = false;
		}
		$nav_urls['specialpages'] = array( 'href' =>

self::makeSpecialUrl( 'Specialpages' ) );
+ $nav_urls['recentchanges'] = array('href' =>
self::makeSpecialUrl('Recentchanges'));

		// A print stylesheet is attached to all pages, but nobody ever

Index: includes/Skin.php

  • includes/Skin.php (revision 16806)

+++ includes/Skin.php (working copy)
@@ -1511,55 +1511,91 @@
function buildSidebar() {

		global $parserMemc, $wgEnableSidebarCache;
		global $wgLang, $wgContLang;

+
+ detect if monobook is set as the current skin
+
may be set to false if there is no MediaWiki:monobooksidebar
+ $isMonobook = $this->skinname == "monobook";

+ // run any hooks

		$fname = 'SkinTemplate::buildSidebar';
		wfProfileIn( $fname );
  • $key = wfMemcKey( 'sidebar' );

+

		$cacheSidebar = $wgEnableSidebarCache &&
			($wgLang->getCode() == $wgContLang->getCode());
  • if ($cacheSidebar) {
  • $cachedsidebar = $parserMemc->get( $key );
  • if ($cachedsidebar!="") {
  • wfProfileOut($fname);
  • return $cachedsidebar;

+ user has enabled sidebar cache
+ if ($cacheSidebar)
+ {
+
if the user is using monobook
+ if($isMonobook)
+ {
+ $key = wfMemcKey( 'monobooksidebar' );
+ if the cache exists, return.
+ if(($cachedsidebar = $parserMemc->get($key))
!== false)
+ {
+ wfProfileOut($fname);
+ return $cachedsidebar;
+ }
+
if MediaWiki:monobooksidebar exists, parse,
if it doesn't exist, try the other cache
+ else if(wfEmptyMsg('monobooksidebar',
($monobooksidebar = wfMsgForContent('monobooksidebar'))))
+ {
+ $isMonobook = false;
+ }

			}

+ if(!$isMonobook)
+ {
+ $key = wfMemcKey( 'sidebar' );
+ if(($cachedsidebar = $parserMemc->get($key))
!== false)
+ {
+ wfProfileOut($fname);
+ return $cachedsidebar;
+ }
+ }

		}
  • $bar = array();
  • $lines = explode( "\n", wfMsgForContent( 'sidebar' ) );
  • foreach ($lines as $line) {

+
+ initialize output array
+ $result = array();
+
+
if the user disabled the sidebar cache, and $monobooksidebar
was never saved
+ if($isMonobook && !isset($monobooksidebar)) { $monobooksidebar

wfMsgForContent('monobooksidebar'); }

+ if($isMonobook && !wfEmptyMsg('monobooksidebar',
$monobooksidebar)) { $lines = explode("\n", $monobooksidebar); }
+ else { $lines = explode("\n", wfMsgForContent('sidebar')); }
+
+ foreach ($lines as $line)
+ {

			if (strpos($line, '*') !== 0)
				continue;
  • if (strpos($line, '**') !== 0) {

+ if (strpos($line, '**') !== 0)
+ {

				$line = trim($line, '* ');
				$heading = $line;
  • } else {
  • if (strpos($line, '|') !== false) { // sanity

check

  • $line = explode( '|' , trim($line, '*

'), 2 );

  • $link = wfMsgForContent( $line[0] );
  • if ($link == '-')
  • continue;
  • if (wfEmptyMsg($line[1], $text =

wfMsg($line[1])))

  • $text = $line[1];
  • if (wfEmptyMsg($line[0], $link))
  • $link = $line[0];
  • $href =

self::makeInternalOrExternalUrl( $link );

  • $bar[$heading][] = array(
  • 'text' => $text,
  • 'href' => $href,
  • 'id' => 'n-' . strtr($line[1],

' ', '-'),

  • 'active' => false
  • );
  • } else { continue; }

+ if($heading == "#searchbox") {
$result[$heading] = 'special:searchbox'; }

			}

+ else if (strpos($line, '|') !== false)
+ {
+ $line = explode( '|' , trim($line, '* '), 2 );
+ $link = wfMsgForContent( $line[0] );
+ if ($link == '-')
+ continue;
+ if (wfEmptyMsg($line[1], $text =
wfMsg($line[1])))
+ $text = $line[1];
+ if (wfEmptyMsg($line[0], $link))
+ $link = $line[0];
+ $href = $this->makeInternalOrExternalUrl( $link
);
+ if($heading == "#searchbox") {
$result[$heading] = array(); }
+ $result[$heading][] = array(
+ 'text' => $text,
+ 'href' => $href,
+ 'id' => 'n-' . strtr($line[1], ' ',
'-'),
+ 'active' => false
+ );
+ }

		}

+

		if ($cacheSidebar)
  • $cachednotice = $parserMemc->set( $key, $bar, 86400 );

+ $cachednotice = $parserMemc->set( $key, $result, 86400
);

		wfProfileOut( $fname );
  • return $bar;

+ return $result;
}
}
?>

gatoatigrado wrote:

would someone delete that? sorry, I meant to delete the "wiki/" prefix. I guess
I have to upload it again.

gatoatigrado wrote:

patch for new sidebar functions

attachment sidebar_diff_2006-10-04.diff ignored as obsolete

gatoatigrado wrote:

is there a php4 version?

Forget it, we're not going to have wiki-specific MonoBook.php files unless
there's a really, really good reason for it. That file used unpatched for all
Wikimedia projects, and it is regularly updated by MediaWiki developers. I don't
want to have to merge conflicts in half a dozen wiki-specific MonoBook.php
files. The required changes can be implemented by changing Skin::buildSidebar()
to allow the user to specify the location of the search box (say with an empty
"* search" section), and by adding a syntax for horizontal rues.

gatoatigrado wrote:

um, okay, sort of took you all a long time to say no. Adding an empty "* search"
section is just as skin-specific. Are you saying you would rather update all of
the skins? I can reapply any changes if you're having merge problems, I understand.

gatoatigrado wrote:

However, I would like your input on how to resolve the problem. Please don't get
defensive about the way it was coded.... There was a clear majority opinion to
change the sidebar.
http://en.wikipedia.org/wiki/Wikipedia_talk:Village_pump_%28proposals%29/Sidebar_redesign/Final_draft_vote#Survey_results_table

gatoatigrado wrote:

"wiki-specific MonoBook.php files" - what about the wiki-specific file for
Wikipedia's Special:Cite page? Is that using hooks? I am open minded to changing
styles or otherwise resolving the problem. I admit that this solution was
something of a hack. Unfortunately I don't have time to work on it right now,
but maybe in two weeks or so.

ayg wrote:

Special:Cite is an extension. Hooks, yes, sort of. I guess you could write an
extension that extends Monobook and overwrites some methods, but that seems
extreme. Probably adding more flexibility to the underlying messages would be
better.

I fail to see how adding an empty "* search" section needs to be skin-specific,
at least if it's optional.

And one last thing: the toolbox is intended to be page-specific. RC, special
pages, and upload links aren't the purpose it's supposed to serve.

gatoatigrado wrote:

The empty "* search" box is skin specific because on skins that do not parse it
a special way, it will end up being a blank box titled "search" instead of the
actual search box.

I know toolbox is obviously page-specific, but why does it need to be skin
specific? I don't think it should be. Compared to the navigational sidebar code,
the skin does a lot of the parsing for the toolbox ... my edits reduced that a
bit, but not very much. I know RC, special pages, etc. should be generated
before, I guess if you think it's worthwhile I could recode it when I have time
(in a few weeks). Are you saying those (as well as Cite) should be somewhere
else visually? I guess from a programmer's perspective it makes sense.

This sounds sort of radical, but what about placing all of the toolbox links
somehow at the top with the tabs? It seems like it would be a positioning
nightmare though. it would end up being a two click process to save space and
probably pretty messy.

I don't understand what you are saying about a syntax for horizontal rules. It
uses the '-' in the foreach loop, which you could obviously change to whatever.

robchur wrote:

(In reply to comment #28)

The empty "* search" box is skin specific because on skins that do not parse it
a special way, it will end up being a blank box titled "search" instead of the
actual search box.

Well, all skins should be calling a single, inherited function to parse the
sidebar page and build the list items as needed, so this should not be a problem.

I know toolbox is obviously page-specific, but why does it need to be skin
specific? I don't think it should be. Compared to the navigational sidebar code,
the skin does a lot of the parsing for the toolbox ... my edits reduced that a
bit, but not very much. I know RC, special pages, etc. should be generated
before, I guess if you think it's worthwhile I could recode it when I have time
(in a few weeks). Are you saying those (as well as Cite) should be somewhere
else visually? I guess from a programmer's perspective it makes sense.

No. The toolbox provides context-sensitive links for tools which might be useful
given the current page scope. So, for articles when the Special:Cite extension
is installed, this adds a "cite this page" link; when viewing a user page as an
administrator, it shows a block link, etc.

The toolbox is *not* for storing links to things like Recent Changes.

This sounds sort of radical, but what about placing all of the toolbox links
somehow at the top with the tabs? It seems like it would be a positioning
nightmare though. it would end up being a two click process to save space and
probably pretty messy.

Forget it.

I don't understand what you are saying about a syntax for horizontal rules. It
uses the '-' in the foreach loop, which you could obviously change to whatever.

He's saying that it's probably worth us introducing some sort of simple syntax
for horizontal rules in the sidebar. He isn't commenting on your code, which we
have established is not actually acceptable for inclusion in MediaWiki.

gatoatigrado wrote:

(In reply to comment #29)

Well, all skins should be calling a single, inherited function to parse the
sidebar page and build the list items as needed, so this should not be a problem.

Yes, and the function returns an array where the keys are the headers and the
values are the bullet points, which contain additional information. Any
exception to move the searchbox would have to be skin specific. Look at
monobook.php - the search code is rather statically placed below the
navigational boxes. If you can further describe how this would work, that would
be fine.

No. The toolbox provides context-sensitive links for tools which might be useful
given the current page scope. So, for articles when the Special:Cite extension
is installed, this adds a "cite this page" link; when viewing a user page as an
administrator, it shows a block link, etc.

The way it is currently implemented the ordering of links, etc. is all
skin-specific.

The toolbox is *not* for storing links to things like Recent Changes.

Forget it.

Or the special pages or links to Special:Upload? How do you think Special:Upload
is more appropriate than Special:Recentchanges?
Yeah, that's fine. I hope you consider the dynamic nature of
Special:Recentchanges and don't immediately disregard all non-page-specific
links in the toolbox, because there already are some.

He's saying that it's probably worth us introducing some sort of simple syntax
for horizontal rules in the sidebar. He isn't commenting on your code, which we
have established is not actually acceptable for inclusion in MediaWiki.

How do you plan on introducing a new syntax when the toolbox is all coded in
PHP? Please be more specific - what you plan to use as input, output. Thanks.

Please do not rail against my code without providing sufficient arguments - and
even if you hate it with good reason, it's possible to introduce that without
being curt and rude. I agree the concept is rather messy, and I would like it if
you would introduce a syntax for the searchbox for all skins. I believe that
without CSS tricks, that would involve editing every skin file (monobook.php,
etc.). In defense of my code, it does include more comments than the original
(http://bugzilla.wikimedia.org/attachment.cgi?id=2442&action=view). On the other
hand, I am open to (polite) criticism, and as much of it as you have.

gatoatigrado wrote:

Forget it.

Right, I agree. Simetrical seemed to think non-page-specific links shouldn't
belong in the toolbox.

ayg wrote:

(In reply to comment #30)

Yes, and the function returns an array where the keys are the headers and the
values are the bullet points, which contain additional information. Any
exception to move the searchbox would have to be skin specific. Look at
monobook.php - the search code is rather statically placed below the
navigational boxes. If you can further describe how this would work, that would
be fine.

I think there's some confusion over "skin-specific". Actually, I don't know how it
came up in the first place, to be honest, since it doesn't seem relevant to anything
anyone was saying. :) Changes that are *skin-specific* are fine if extending them
to other skins doesn't make sense (i.e., don't do "giving shiny new features to
Monobook users only", but "tweaking layout of Monobook only" is fine), although we
don't want to greatly change the look of any currently existing style (make a new
style for that).

Or the special pages or links to Special:Upload? How do you think Special:Upload
is more appropriate than Special:Recentchanges?

Good question. What *is* the toolbox for? Hmm. Well, note that the toolbox
definitely can't be edited from MediaWiki:Sidebar, because it's only in the sidebar
in Monobook. I don't like the idea of making a totally different sidebar message
for Monobook, either, because that requires people to keep two substantially similar
sidebar messages up to date, which they won't and shouldn't have to.

Maybe make a MediaWiki:Toolbox that only works for Monobook, and comparable messages
for other skins. I dunno.

How do you plan on introducing a new syntax when the toolbox is all coded in
PHP? Please be more specific - what you plan to use as input, output. Thanks.

Input in MediaWiki:Sidebar, e.g., a single line containing only "----". Output
to HTML, <hr />. Like the wikisyntax.

Please do not rail against my code without providing sufficient arguments - and
even if you hate it with good reason, it's possible to introduce that without
being curt and rude.

A few major issues with it that I spotted:

  1. Skin.php shouldn't know about Monobook. Any Monobook-specific changes to Skin

methods should be performed by just overriding them in Monobook.php.

  1. Requiring admins to maintain separate sidebars for Monobook and non-Monobook

is inefficient and will lead to confusion.

  1. Your changes do not appear to be reverse-compatible, in that customizations to

MediaWiki:Sidebar will no longer appear in Monobook.

  1. You're combining basically three separate issues (customize toolbox, allow

horizontal rules, allow stuff below search box) into one bug and one patch.

gatoatigrado wrote:

I have one major question before I do some more work with the code (which may
have to be on the weekend or later) - are the usual skins the only ones I have
to worry about? Because there will be some modification necessary to do what you
are proposing.

(In reply to comment #32)

I think there's some confusion over "skin-specific". Actually, I don't know

how it

came up in the first place, to be honest, since it doesn't seem relevant to

anything

anyone was saying. :) Changes that are *skin-specific* are fine if extending

them

to other skins doesn't make sense (i.e., don't do "giving shiny new features to
Monobook users only", but "tweaking layout of Monobook only" is fine), although we
don't want to greatly change the look of any currently existing style (make a new
style for that).

(i.e., don't do "giving shiny new features to Monobook users only", but
"tweaking layout of Monobook only" is fine) - right. Okay, I'll modify them all,
but it this project certainly requires modifying them all. Unless we are to rely
on CSS tricks, but I am against using CSS excessively to correct bad HTML output.

Good question. What *is* the toolbox for? Hmm. Well, note that the toolbox
definitely can't be edited from MediaWiki:Sidebar, because it's only in the

sidebar

in Monobook. I don't like the idea of making a totally different sidebar message
for Monobook, either, because that requires people to keep two substantially

similar

sidebar messages up to date, which they won't and shouldn't have to.

Maybe make a MediaWiki:Toolbox that only works for Monobook, and comparable

messages

for other skins. I dunno.

um, I liked your idea of editing sidebar more than a MediaWiki:Toolbox. This is
what I think,

  • Navigation
    • Link 1|Title
  • Search

{{#searchbox}}

  • Toolbox

{{#page specific toolbox}}

  • Special:Upload, etc.

The exact syntax could be changed later...I think this might work

buildSidebar() // cached
array {

[header] { (link)) }
[search_header] { (searchbox) }
[toolbox_header] { (pagelinks) (hr) (link) } }

buildCurrentPageSidebar() // same for all skins
array {

[header] { (link) }
[search_header] { (searchbox) }
[toolbox_header] { (link) (hr) (link) } }

skin builds sidebar with all of this, echo search box html function similar to
the original code. The greyed out "permanent link" for specified "old id" pages
would need to be implemented somehow. But I think a more unified sidebar is a
good idea.

Input in MediaWiki:Sidebar, e.g., a single line containing only "----". Output
to HTML, <hr />. Like the wikisyntax.

A few major issues with it that I spotted:

  1. Skin.php shouldn't know about Monobook. Any Monobook-specific changes to Skin

methods should be performed by just overriding them in Monobook.php.

  1. Requiring admins to maintain separate sidebars for Monobook and non-Monobook

is inefficient and will lead to confusion.

  1. Your changes do not appear to be reverse-compatible, in that customizations to

MediaWiki:Sidebar will no longer appear in Monobook.

  1. You're combining basically three separate issues (customize toolbox, allow

horizontal rules, allow stuff below search box) into one bug and one patch.

Okay, thanks a lot. This is much nicer and helpful <smiley face here lol>. I
most certainly agree.

ayg wrote:

(In reply to comment #33)

I have one major question before I do some more work with the code (which may
have to be on the weekend or later) - are the usual skins the only ones I have
to worry about? Because there will be some modification necessary to do what you
are proposing.

There's no reason to go around breaking custom skins gratuitously, but it's not
a big deal if you do. Just you may as well keep it compatible with custom skins
to the extent possible, and it does have to work with all the skins that ship
with MW.

(i.e., don't do "giving shiny new features to Monobook users only", but
"tweaking layout of Monobook only" is fine) - right. Okay, I'll modify them all,
but it this project certainly requires modifying them all. Unless we are to rely
on CSS tricks, but I am against using CSS excessively to correct bad HTML output.

I would imagine it would only require modifying MonoBook.php and Skin.php, or
something to that effect, since all the other skins inherit from one or the
other IIRC, so anything that inherits from either should be okay.

um, I liked your idea of editing sidebar more than a MediaWiki:Toolbox. This is
what I think,

  • Navigation
    • Link 1|Title
  • Search

{{#searchbox}}

  • Toolbox

{{#page specific toolbox}}

  • Special:Upload, etc.

But non-Monobook-based skins don't *have* a toolbox. What do you want them to
do with that extra stuff they loaded from MediaWiki:Sidebar?

http://en.wikipedia.org/wiki/Main_Page?useskin=cologneblue

gatoatigrado wrote:

Okay, I recoded the searchbox code, hopefully it's okay. cologne blue has a very
different sidebar. It only reads the first sidebar box anyway. I think this
needs to be changed. I don't want to mess around uploading a temporary patch
here, so I put it on my website. It has screenshots from monobook and cologne
blue. I'm obviously not done yet; cologne blue needs to have a more customizable
sidebar. anyway, tell me what you think. thanks.

The searchbox building code is more centralized now. I took out the break for
the cologne blue searchbox at the end of the page because I thought it looked
bad--the buttons are on the next row, but the input field is at the end of the
previous row.

http://wiki.ntung.com/images/3/33/Sidebar.tar.bz2

(In reply to comment #34)

I have one major question before I do some more work with the code (which may
have to be on the weekend or later) - are the usual skins the only ones I have
to worry about? Because there will be some modification necessary to do what you
are proposing.

right, in cologne blue it ignores any text that is equal to '-', so I can set
that for the searchbox and horizontal rule elements

I would imagine it would only require modifying MonoBook.php and Skin.php, or
something to that effect, since all the other skins inherit from one or the
other IIRC, so anything that inherits from either should be okay.

Not quite. Monobook extends quick template, and cologneblue extends Skin. they
are quite different.

But non-Monobook-based skins don't *have* a toolbox. What do you want them to
do with that extra stuff they loaded from MediaWiki:Sidebar?

oh, yeah, I'm new, sorry. see comments above.

robchur wrote:

We require a patch to be attached to this bug report. How do you expect us to
review or even accept your code if we can't see a nice, simple patch? URLs to
external web pages invariably don't last forever.

gatoatigrado wrote:

it's not done, that's why I have a link. Perhaps you can suggest a way to
involve the toolbox, and to help make this skin-compliant. I don't want to
program a bunch of junk that you (or anyone) would just reject.

gatoatigrado wrote:

patch for new sidebar functions

does not include toolbox modifications. removed some repetitive code.

Attached:

gatoatigrado wrote:

the standard skin intentionally does not display the horizontal lines or the
searchbox.

robchur wrote:

I've committed some changes on a branch which make it possibly simply to specify
items that appear in the navigation bar which appear below the search segment.
This is achieved through editing a new message, "sidebar2" - the existing build
methods and caching are preserved.

gatoatigrado wrote:

I fail to see how mine doesn't preserve build methods and caching, but if you're
happier working with your own code, that's fine. As long as it works--which it
did for me.

I have a few minor concerns with your code--it increases repeated code, and
doesn't look the same in all skins. The echo sidebar in monobook is almost a
copy of the code that does the same thing above. The other skins don't show it,
which could be bad in Wikipedia because there are a few possibly important links

  • "Help / Community portal / Questions / Contact us / Donations" which wouldn't

show up for other skins.

Otherwise, yours does allow the existing MediaWiki:Sidebar not to be changed
(the search box does not have to be added). I could add that in the buildSidebar
function, if it's important.

robchur wrote:

It's not that yours didn't, it's just that I didn't agree with needing to allow
the search box to be moved around anyway.

My code isn't done; the commit messages alone should have given you that
impression; adding support for it in MonoBook was a quick hack, hence the
repetition, and I haven't got around to working on adding it cleanly to all
skins. It was more of a "look, it can be done fairly simply" job than a, "look,
I've done it" sort of job.

gatoatigrado wrote:

right, that's fine. Yours is a lot simpler. Perhaps you can copy part of what I
did for cologne blue, minus the horizontal lines and searchbox edits. It just
goes through the sidebar headers and echoes them instead of the "qbbrowse"
message. Perhaps you can concatenate the sidebar2 for cologne blue because
that's below the searchbox anyway. You'll need to modify standard.php also. The
other skins should use the sidebar from monobook or cologne blue.

Should I create a new bug for the toolbox, or is that too trivial? The <hr>
looks nice--http://wiki.ntung.com.

gatoatigrado wrote:

I don't know if your code will be more simple when it is done... the main reason
for the large diff in mine is that I consolidated a lot of the build searchbox
functions to skin.php.

I suppose you're right that it doesn't need to be moved. Would you like me to
patch the other skins from your trunk?

I'm going to come out and say we're not going to support customizing the location of the search box within the side bar; in part because it probably shouldn't be in the side bar. Long term we'll probably be moving it to a more consistent location.

This has been FIXED with [[rev:37232]] (http://svn.wikimedia.org/viewvc/mediawiki?view=rev&revision=37232): Log Message: Allow the search box, toolbox and languages box in the Monobook sidebar to be moved around arbitrarily using special sections in [[MediaWiki:Sidebar]]: SEARCH, TOOLBOX and LANGUAGES