Last modified: 2011-07-19 16:28:54 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T14261, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 12261 - Render category links as an HTML list
Render category links as an HTML list
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
unspecified
All All
: Low enhancement with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
http://lumino.us/weblog/pipe-dream
: patch
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-12-09 22:58 UTC by Andrew Dunbar
Modified: 2011-07-19 16:28 UTC (History)
6 users (show)

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


Attachments
inline unordered list for catlinks (4.88 KB, patch)
2010-03-18 03:48 UTC, Thana
Details
try again (4.84 KB, patch)
2010-03-18 03:58 UTC, Thana
Details

Description Andrew Dunbar 2007-12-09 22:58:06 UTC
Currently the category links section is rendered by a series of links separated by explicit pipe characters "|".

This makes CSS / JavaScript category link filtering extremely difficult since if we use styles to hide certain links the pipes between them will still be visible.

With a pure CSS solution such as "Pipe Dream" detailed at http://lumino.us/weblog/pipe-dream all we need to do is set display:none to any link we wish to hide and the pipes "just work".
Comment 1 Thana 2010-03-18 03:48:35 UTC
Created attachment 7215 [details]
inline unordered list for catlinks

This patch would appear to make the system message "catseparator" obsolete, and duly removes it.
Comment 2 Thana 2010-03-18 03:51:27 UTC
Also it fails in cologneblue, nostalgia, etc. I figured out it was because they do not produce a div with id="catlinks", so that might also be worth fixing.
Comment 3 Thana 2010-03-18 03:56:18 UTC
Comment on attachment 7215 [details]
inline unordered list for catlinks

Index: maintenance/language/messages.inc
===================================================================
--- maintenance/language/messages.inc	(revision 63894)
+++ maintenance/language/messages.inc	(working copy)
@@ -2891,7 +2891,6 @@
 		'confirm-purge-bottom',
 	),
 	'separators' => array(
-		'catseparator',
 		'semicolon-separator',
 		'comma-separator',
 		'colon-separator',
Index: maintenance/language/messageTypes.inc
===================================================================
--- maintenance/language/messageTypes.inc	(revision 63894)
+++ maintenance/language/messageTypes.inc	(working copy)
@@ -330,7 +330,6 @@
 	'hebrew-calendar-m11-gen',
 	'hebrew-calendar-m12-gen',
 	'version-svn-revision',
-	'catseparator',
 	'semicolon-separator',
 	'comma-separator',
 	'colon-separator',
Index: skins/common/shared.css
===================================================================
--- skins/common/shared.css	(revision 63894)
+++ skins/common/shared.css	(working copy)
@@ -829,3 +829,21 @@
 a.sortheader {
 	margin: 0 0.3em;
 }
+
+#catlinks ul, li {
+	display:inline;
+	margin:0px;
+	padding:0px;
+	list-style:none;
+	list-style-type:none;
+	list-style-image:none;
+	}
+
+#catlinks li:after {
+	content: " | ";
+	}
+		
+#catlinks li:last-child:after {
+	content: "";
+	}
+
Index: includes/Skin.php
===================================================================
--- includes/Skin.php	(revision 63894)
+++ includes/Skin.php	(working copy)
@@ -779,25 +779,21 @@
 			return '';
 		}
 
-		# Separator
-		$sep = wfMsgExt( 'catseparator', array( 'parsemag', 'escapenoentities' ) );
-
 		// Use Unicode bidi embedding override characters,
 		// to make sure links don't smash each other up in ugly ways.
 		$dir = $wgContLang->getDir();
-		$embed = "<span dir='$dir'>";
-		$pop = '</span>';
+		$embed = "<li dir='$dir'>";
+		$pop = '</li>';
 
 		$allCats = $wgOut->getCategoryLinks();
 		$s = '';
 		$colon = wfMsgExt( 'colon-separator', 'escapenoentities' );
 		if ( !empty( $allCats['normal'] ) ) {
-			$t = $embed . implode( "{$pop} {$sep} {$embed}" , $allCats['normal'] ) . $pop;
-
 			$msg = wfMsgExt( 'pagecategories', array( 'parsemag', 'escapenoentities' ), count( $allCats['normal'] ) );
-			$s .= '<div id="mw-normal-catlinks">' .
-				$this->link( Title::newFromText( wfMsgForContent( 'pagecategorieslink' ) ), $msg )
-				. $colon . $t . '</div>';
+			$s .= '<div id="mw-normal-catlinks">'
+				. $this->link( Title::newFromText( wfMsgForContent( 'pagecategorieslink' ) ), $msg ). $colon 
+				. '<ul>' . $embed . implode( "{$pop} {$embed}" , $allCats['normal'] ) . $pop . '</ul>'
+				. '</div>';
 		}
 
 		# Hidden categories
@@ -809,10 +805,10 @@
 			} else {
 				$class = 'mw-hidden-cats-hidden';
 			}
-			$s .= "<div id=\"mw-hidden-catlinks\" class=\"$class\">" .
-				wfMsgExt( 'hidden-categories', array( 'parsemag', 'escapenoentities' ), count( $allCats['hidden'] ) ) .
-				$colon . $embed . implode( "$pop $sep $embed", $allCats['hidden'] ) . $pop .
-				'</div>';
+			$s .= "<div id=\"mw-hidden-catlinks\" class=\"$class\">"
+				. wfMsgExt( 'hidden-categories', array( 'parsemag', 'escapenoentities' ), count( $allCats['hidden'] ) ) . $colon
+				. '<ul>' . $embed . implode( "{$pop} {$embed}" , $allCats['hidden'] ) . $pop . '</ul>'
+				. '</div>';
 		}
 
 		# optional 'dmoz-like' category browser. Will be shown under the list
Index: languages/messages/MessagesEn.php
===================================================================
--- languages/messages/MessagesEn.php	(revision 63894)
+++ languages/messages/MessagesEn.php	(working copy)
@@ -506,7 +506,6 @@
 	'accesskey-t-specialpages',
 	'accesskey-t-whatlinkshere',
 	'anonnotice',
-	'catseparator',
 	'colon-separator',
 	'currentevents',
 	'currentevents-url',
@@ -3952,7 +3951,6 @@
 'confirm-purge-bottom' => 'Purging a page clears the cache and forces the most current revision to appear.',
 
 # Separators for various lists, etc.
-'catseparator'        => '|', # only translate this message to other languages if you have to change it
 'semicolon-separator' => ';&#32;', # only translate this message to other languages if you have to change it
 'comma-separator'     => ',&#32;', # only translate this message to other languages if you have to change it
 'colon-separator'     => ':&#32;', # only translate this message to other languages if you have to change it
Index: languages/messages/MessagesKrc.php
===================================================================
--- languages/messages/MessagesKrc.php	(revision 63894)
+++ languages/messages/MessagesKrc.php	(working copy)
@@ -2997,7 +2997,6 @@
 'confirm-purge-bottom' => 'Бетни кеши кетерилгенден сора, андан сора келген версиясы кёргюзюллюкдю.',
 
 # Separators for various lists, etc.
-'catseparator'        => '|',
 'semicolon-separator' => ';&#32;',
 'percent'             => '$1%',
 'parentheses'         => '($1)',
Comment 4 Thana 2010-03-18 03:56:55 UTC
i give up trying to edit the patch.
Comment 5 Thana 2010-03-18 03:58:09 UTC
Created attachment 7216 [details]
try again
Comment 6 Bawolff (Brian Wolff) 2010-03-18 03:59:33 UTC
I believe the patch would not work in IE. (as IE7 and lower don't support after psuedo-element. no version of IE supports the last-child pseudo-selector, and safari 3 for windows also doesn't support last-child selector - http://www.quirksmode.org/css/contents.html ). Probably would degrade relativley gracefully though.
Comment 7 Thana 2010-03-18 04:13:23 UTC
Could someone kindly remove comment #3? I did not mean to dump it there but was rather trying to edit the patch which seems to have attached itself before I clicked the submit. Very sorry about that.

(In reply to comment #6)
> I believe the patch would not work in IE. (as IE7 and lower don't support after
> psuedo-element. no version of IE supports the last-child pseudo-selector, and
> safari 3 for windows also doesn't support last-child selector -
> http://www.quirksmode.org/css/contents.html ). Probably would degrade
> relativley gracefully though.

So if I understand correctly it would have an extra separator after the last element. Didn't have IE handy to test with but I actually figured it would do worse than that.

I think the key issue is being able to select and copy the content (list of categories) with the cursor without getting the presentational bits.
Comment 8 Bawolff (Brian Wolff) 2010-03-18 04:28:00 UTC
(In reply to comment #7)
> Could someone kindly remove comment #3? I did not mean to dump it there but was
> rather trying to edit the patch which seems to have attached itself before I
> clicked the submit. Very sorry about that.

Comments (to my knowledge) cannot be removed  from bugzilla. (combined with the fact there is no preview, its a really annoying part of bugzilla)

> 
> So if I understand correctly it would have an extra separator after the last
> element. Didn't have IE handy to test with but I actually figured it would do
> worse than that.
> 
I think it would have the extra separators in IE8, and in IE7 and lower it would have no separators (this is just a guess, for all i know it could totally blow up in IE). However considered the current method works perfectly fine, in my humble opinion, I think it should stay that way (however it should be noted my opinion does not have any weight for what will or will not happen).
Comment 9 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-03-18 16:24:35 UTC
This patch would fail pretty badly in IE < 8 -- the separators would not appear.  Since IE < 8 is still such a large chunk of our viewers, this is an unacceptable regression given the limited benefits.

I'm not convinced that separators should be part of CSS rather than content.  I'm not even sure <ul> is best here.  Copy-paste into a WYSIWYG editor would be pretty surprising -- currently you'd get roughly what you see, but using a <ul>, it would magically transform into a bulleted list.  <ul> is normally used for block-level lists, not inline lists.

Finally, please try to observe MediaWiki style conventions when submitting patches.  Just imitating surrounding code would be a good start -- closing brace is not indented, space between colon and property value.
Comment 10 Bergi 2010-09-07 12:31:22 UTC
I always thought about that bug, but it has low importance (OK, it might make Hotcat to be faster).
In my Webpages I usually use <ul> for any lists, even inline ones, because it is the designated tag for that. I also like these vertical bars between the items, but today I'm no more doing the mistake to use :after{content:"|"} in my CSS for that.
This phrase would make the | be selectable content, which it should be not; being just a matter of design.
It should be done with a simple border instead:
#catlinks <one item> {
   display: inline;
   display: inline-block;
   padding: 0 1em; /*some gap*/
   border-left: 1px solid #000;
}
#catlinks <one item>:first-child {
   padding-left: 0;
   border-left: none;
}
Of course we don't have to use ul>li, this also could be div>span or something else, but then old browser which do not understand the inline-block together with the margin wont display any space between.
Comment 11 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-09-08 17:20:48 UTC
Browsers do not generally treat generated content as selectable, so that's not a problem (or are there browsers that disagree?).  Using borders would be fine, but IE6 doesn't support :first-child, so some hack would be needed unless we think this is minor enough that IE6 users can suffer.
Comment 12 Bergi 2010-09-11 09:42:45 UTC
Opera does, I dont know whether it is done by others.
A hack for IE could be adding some class="first-child".
Comment 13 p858snake 2011-04-30 00:10:16 UTC
*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*
Comment 14 DieBuche 2011-07-13 10:43:02 UTC
Modified patch ( Thana's patch + Bergi's border trick + some changes to Nostalgia & CologneBlue ) applied in r92054
Comment 15 Bergi 2011-07-19 16:28:54 UTC
Thanks. I just wanted to start arguing for the changes already made in r92245 :-)

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


Navigation
Links