Last modified: 2008-08-01 05:35:04 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 7493 - Syntax to customise location of search box within navigation bar
Syntax to customise location of search box within navigation bar
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
unspecified
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-10-04 20:39 UTC by Nicholas Tung
Modified: 2008-08-01 05:35 UTC (History)
1 user (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
diff for skin template (3.63 KB, patch)
2006-10-04 21:19 UTC, Nicholas Tung
Details
diff for skin (3.63 KB, patch)
2006-10-04 21:19 UTC, Nicholas Tung
Details
diff for monobook (4.65 KB, patch)
2006-10-04 21:20 UTC, Nicholas Tung
Details
patch for new sidebar functions (10.14 KB, patch)
2006-10-04 23:44 UTC, Nicholas Tung
Details
patch for new sidebar functions (10.10 KB, patch)
2006-10-04 23:57 UTC, Nicholas Tung
Details
patch for new sidebar functions (10.68 KB, patch)
2006-12-20 16:14 UTC, Nicholas Tung
Details

Description Nicholas Tung 2006-10-04 20:39:08 UTC
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
Comment 1 Brion Vibber 2006-10-04 20:42:19 UTC
Linked page neither explains what it does nor includes any code.
What on earth is this?
Comment 2 Nicholas Tung 2006-10-04 20:45:20 UTC
Follow the User:Gatoatigrado/sidebarhack link. The project page is at
http://en.wikipedia.org/wiki/Wikipedia:Village_pump_%28proposals%29/Sidebar_redesign.
Comment 3 Nicholas Tung 2006-10-04 20:45:53 UTC
Brion I sent you a message on your wikipedia talk page explaining all of
this...I guess you didn't read it.
Comment 4 Nicholas Tung 2006-10-04 20:55:05 UTC
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.
Comment 5 Brion Vibber 2006-10-04 20:56:24 UTC
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?
Comment 6 Nicholas Tung 2006-10-04 21:01:59 UTC
"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.
Comment 7 Brion Vibber 2006-10-04 21:03:18 UTC
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.
Comment 8 Nicholas Tung 2006-10-04 21:13:03 UTC
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.
Comment 9 Nicholas Tung 2006-10-04 21:19:06 UTC
Created attachment 2441 [details]
diff for skin template
Comment 10 Nicholas Tung 2006-10-04 21:19:45 UTC
Created attachment 2442 [details]
diff for skin
Comment 11 Nicholas Tung 2006-10-04 21:20:07 UTC
Created attachment 2443 [details]
diff for monobook
Comment 12 Nicholas Tung 2006-10-04 21:23:16 UTC
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).
Comment 13 Nicholas Tung 2006-10-04 21:25:27 UTC
"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.
Comment 14 Brion Vibber 2006-10-04 21:25:40 UTC
1) Always use unified diffs, which are more readable and can be applied to
modified files more reliably. Simply issue 'svn diff' for this.

2) 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.

3) Put all patches that go together in one file and one attachment.

4) Mark the attachments as patches in bugzilla so they can be read online.
Comment 15 Nicholas Tung 2006-10-04 21:40:36 UTC
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.
Comment 16 Brion Vibber 2006-10-04 21:41:57 UTC
"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
Comment 17 Nicholas Tung 2006-10-04 23:44:07 UTC
wow that was a lot easier than I thought it would be. Worked fine for me. Thanks.
Comment 18 Nicholas Tung 2006-10-04 23:44:49 UTC
Created attachment 2444 [details]
patch for new sidebar functions
Comment 19 Nicholas Tung 2006-10-04 23:55:27 UTC
Comment on attachment 2441 [details]
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;
	}
 }
 ?>
Comment 20 Nicholas Tung 2006-10-04 23:56:40 UTC
would someone delete that? sorry, I meant to delete the "wiki/" prefix. I guess
I have to upload it again.
Comment 21 Nicholas Tung 2006-10-04 23:57:21 UTC
Created attachment 2445 [details]
patch for new sidebar functions
Comment 22 Nicholas Tung 2006-10-05 00:06:16 UTC
is there a php4 version?
Comment 23 Tim Starling 2006-12-03 06:00:47 UTC
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. 
Comment 24 Nicholas Tung 2006-12-03 06:41:40 UTC
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.
Comment 25 Nicholas Tung 2006-12-03 06:45:25 UTC
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
Comment 26 Nicholas Tung 2006-12-03 06:48:08 UTC
"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.
Comment 27 Aryeh Gregor (not reading bugmail, please e-mail directly) 2006-12-03 07:00:12 UTC
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.
Comment 28 Nicholas Tung 2006-12-03 08:24:54 UTC
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.
Comment 29 Rob Church 2006-12-03 12:13:34 UTC
(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.
Comment 30 Nicholas Tung 2006-12-03 16:45:22 UTC
(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.
Comment 31 Nicholas Tung 2006-12-03 16:46:58 UTC
> Forget it.
Right, I agree. Simetrical seemed to think non-page-specific links shouldn't
belong in the toolbox.
Comment 32 Aryeh Gregor (not reading bugmail, please e-mail directly) 2006-12-03 17:34:18 UTC
(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.

2) Requiring admins to maintain separate sidebars for Monobook and non-Monobook
is inefficient and will lead to confusion.

3) Your changes do not appear to be reverse-compatible, in that customizations to
MediaWiki:Sidebar will no longer appear in Monobook.

4) You're combining basically three separate issues (customize toolbox, allow
horizontal rules, allow stuff below search box) into one bug and one patch.
Comment 33 Nicholas Tung 2006-12-06 17:37:02 UTC
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.
> 
> 2) Requiring admins to maintain separate sidebars for Monobook and non-Monobook
> is inefficient and will lead to confusion.
> 
> 3) Your changes do not appear to be reverse-compatible, in that customizations to
> MediaWiki:Sidebar will no longer appear in Monobook.
> 
> 4) 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.

Comment 34 Aryeh Gregor (not reading bugmail, please e-mail directly) 2006-12-07 03:10:44 UTC
(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
Comment 35 Nicholas Tung 2006-12-08 18:48:11 UTC
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.
Comment 36 Rob Church 2006-12-08 21:12:05 UTC
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.
Comment 37 Nicholas Tung 2006-12-09 00:56:12 UTC
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.
Comment 38 Nicholas Tung 2006-12-20 16:14:42 UTC
Created attachment 2912 [details]
patch for new sidebar functions

does not include toolbox modifications. removed some repetitive code.
Comment 39 Nicholas Tung 2006-12-20 16:17:44 UTC
the standard skin intentionally does not display the horizontal lines or the
searchbox.
Comment 40 Rob Church 2006-12-20 16:28:57 UTC
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.
Comment 41 Nicholas Tung 2006-12-20 17:22:59 UTC
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.

Comment 42 Rob Church 2006-12-20 17:55:29 UTC
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.
Comment 43 Nicholas Tung 2006-12-21 02:25:45 UTC
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.
Comment 44 Nicholas Tung 2006-12-29 23:11:01 UTC
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?
Comment 45 Brion Vibber 2008-03-18 23:34:59 UTC
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.
Comment 46 Quiddity 2008-08-01 05:35:04 UTC
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


Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links