Last modified: 2011-07-19 16:28:54 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".
Created attachment 7215 [details] inline unordered list for catlinks This patch would appear to make the system message "catseparator" obsolete, and duly removes it.
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 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' => '; ', # only translate this message to other languages if you have to change it 'comma-separator' => ', ', # only translate this message to other languages if you have to change it 'colon-separator' => ': ', # 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' => '; ', 'percent' => '$1%', 'parentheses' => '($1)',
i give up trying to edit the patch.
Created attachment 7216 [details] try again
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.
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.
(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).
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.
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.
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.
Opera does, I dont know whether it is done by others. A hack for IE could be adding some class="first-child".
*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*
Modified patch ( Thana's patch + Bergi's border trick + some changes to Nostalgia & CologneBlue ) applied in r92054
Thanks. I just wanted to start arguing for the changes already made in r92245 :-)