Last modified: 2013-10-23 18:17:42 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 T37371, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 35371 - Sidebar generates duplicate IDs
Sidebar generates duplicate IDs
Status: NEW
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
1.18.x
All All
: Low trivial (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks: html
  Show dependency treegraph
 
Reported: 2012-03-20 23:54 UTC by Porter21
Modified: 2013-10-23 18:17 UTC (History)
5 users (show)

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


Attachments

Description Porter21 2012-03-20 23:54:43 UTC
If two (or more) links in the sidebar have the same display text (even if they point to different pages), the HTML IDs generated for their containing list items will be identical - which they shouldn't be according to definition (IDs should always be unique on a page).

Example:
========

In MediaWiki:Sidebar, add:
* Example section
** Some link|Link text
* Example section 2
** Some other link|Link text

Example output:
===============

<!-- Example section -->
<div id="p-Example_section" class="portal">
	<h5>Example section</h5>
	<div class="body">
		<ul>
			<li id="n-Link-text"><a href="/mediawiki-1.18.1/index.php/Some_link">Link text</a></li>
		</ul>
	</div>
</div>

<!-- /Example section -->

<!-- Example section 2 -->
<div id="p-Example_section_2" class="portal">
	<h5>Example section 2</h5>
	<div class="body">
		<ul>
			<li id="n-Link-text"><a href="/mediawiki-1.18.1/index.php/Some_other_link">Link text</a></li>
		</ul>
	</div>
</div>

<!-- /Example section 2 -->

The generated ID for both list items is "n-Link-text".
Comment 1 Bawolff (Brian Wolff) 2012-03-20 23:59:58 UTC
confirmed. Thank you for the report.


I'm setting priority to low since its an html validation/correctness issue (It
doesn't actively cause problems), and the likelyhood of someone having two
entries with the same display text is low.
Comment 2 MZMcBride 2012-03-21 00:00:58 UTC
The general idea with the sidebar is that you'll use MediaWiki pages for the link and the link text. So it'd be something like:

* Example section
** bloglink|bloglink-text

Where MediaWiki:Bloglink and MediaWiki:Bloglink-text contain the link and the link text, respectively. As all full page titles (namespace + page title) are unique in MediaWiki, there wouldn't be any collisions in CSS IDs.

The sidebar syntax kind of sucks, though. Very few people really understand it and most of them do what you're doing (insert the text directly into MediaWiki:Sidebar rather than splitting it out into messages).
Comment 3 MZMcBride 2012-03-21 00:04:58 UTC
(In reply to comment #2)
> The general idea with the sidebar is that you'll use MediaWiki pages for the
> link and the link text. So it'd be something like:
> 
> * Example section
> ** bloglink|bloglink-text

Though, as Bawolff kind of notes, if you use bloglink-text more than once in the sidebar, you'll still have duplicate IDs. It'd be kind of daft to do this, though.
Comment 4 Bawolff (Brian Wolff) 2012-03-21 00:08:02 UTC
I don't think using text directly there is wrong. I've always felt that the
MediaWiki:Bloglink-text thing was more a hack so that the sidebar could be translated sanely with
the content/user lang distinction. For a wiki that doesn't have to care about
that, its perfectly valid just to put text instead of the name of a message
there imho.
Comment 5 Porter21 2012-03-21 00:38:50 UTC
Why would it be "kind of daft" to use the same interface message if you want to have the same text? It's a bit like saying you should create a new interface message for every instance of "Error" instead of using the same message for each instance.

As for "you're doing it wrong" (to paraphrase the argument), the MediaWiki manual itself simply says: "The link text can be the name of an interface message (page in the MediaWiki namespace) or plain text". It does not state any of these is preferred or might cause issues, so I don't really see how this is a case of someone doing something wrong. Besides, the plain text method is arguably the more commonly used option outside of Wikimedia wikis; on smaller, single-language wikis it's simply cumbersome to use interface messages when you won't have someone to translate them into other languages anyway.

Regarding this being a bit of an edge case, I agree. It does have use cases though; for example, if you have a wiki centered on a limited number of topics, it's not really out-of-the-way that someone would make a sidebar section for each topic and put an "Overview" link in each.
Comment 6 Daniel Friesen 2012-03-21 00:55:09 UTC
That overview case actually shouldn't cause this. Last I checked ids are based off of the link portion of the text not the text portion. So this should only show up if the link is the same. Naturally an "Overview" link won't link to the same page for each overview section so that won't cause a bug.

Personally I feel like just ignoring this. The real ideal plan is to completely ditch our cruddy sidebar syntax and replace it with a nice ui for controlling navigation and some sort of widget block system to integrate those navigation blocks into the sidebar along with the other things we put there. Our sidebar dependence is a crutch that stops us from allowing wikis to use navigation that actually fits instead of shoehorning it into a fixed sidebar.

When we fix that and switch to an interface we can ditch this whole issue. You don't 'really' need an id for every single item like this. We only have it because sometimes you want to target one of them and the only way we can allow that is to put ids on all of them. With a proper ui based system we can default to having no IDs (but implicitly give imported links the same ids as before) and let you set your own custom id on links that actually need an id.
Comment 7 MZMcBride 2012-03-21 01:07:21 UTC
(In reply to comment #6)
> Personally I feel like just ignoring this. The real ideal plan is to completely
> ditch our cruddy sidebar syntax and replace it with a nice ui for controlling
> navigation and some sort of widget block system to integrate those navigation
> blocks into the sidebar along with the other things we put there. Our sidebar
> dependence is a crutch that stops us from allowing wikis to use navigation that
> actually fits instead of shoehorning it into a fixed sidebar.

My general view as well. This is tracked by bug 16943, for reference.
Comment 8 Bawolff (Brian Wolff) 2012-03-21 01:36:06 UTC
>My general view as well. This is tracked by bug 16943, for reference.

We should have a tracking bug for pipe dreams :P


----

>That overview case actually shouldn't cause this. Last I checked ids are based
>off of the link portion of the text not the text portion

That would appear to be incorrect (at least on trunk). Although logically that would make more sense [I suppose the default link message names all have url in them which would look weird in the id].
Comment 9 Porter21 2012-03-21 01:47:54 UTC
(In reply to comment #8)
> >That overview case actually shouldn't cause this. Last I checked ids are based
> >off of the link portion of the text not the text portion
> 
> That would appear to be incorrect (at least on trunk). Although logically that
> would make more sense [I suppose the default link message names all have url in
> them which would look weird in the id].

It's also incorrect for the MW 1.17 and MW 1.18 release versions at least.
Comment 10 Krinkle 2012-03-22 09:55:21 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > >That overview case actually shouldn't cause this. Last I checked ids are based
> > >off of the link portion of the text not the text portion
> > 
> > That would appear to be incorrect (at least on trunk). Although logically that
> > would make more sense [I suppose the default link message names all have url in
> > them which would look weird in the id].
> 
> It's also incorrect for the MW 1.17 and MW 1.18 release versions at least.

I'm not sure wether the ID should be derived from the link or text, but it should certainly not be based on the link text _value_, because then it would have a different ID in different user languages, making it a complete mess to hook into them from CSS or JavaScript.

Currently (1.19wmf1) they appear to be based on the message-key of the link text. So that's not too bad. However in the case where no message-key is used (and it falls back to the raw literal value), then it does end up literally in the ID.

> development
> mw-bugzilla-url|bugtracker
> Special:Code/MediaWiki|codereview
> Special:Code/MediaWiki/comments|codecomments
> browse svn-url|browse svn
> svn statistics-url|svn statistics
> phpdoc-url|phpdoc

<!-- development -->
<div class="portal" id='p-development'>
<h5>Development</h5>
<div class="body">
<ul>
	<li id="n-bugtracker"><a href="/wiki/Bugzilla">Bug tracker</a></li>
	<li id="n-codereview"><a href="/wiki/Special:Code/MediaWiki">View code changes</a></li>
	<li id="n-codecomments"><a href="/wiki/Special:Code/MediaWiki/comments">Code comments</a></li>
	<li id="n-browse-svn"><a href="https://svn.wikimedia.org/viewvc/mediawiki/trunk/">Browse SVN</a></li>
	<li id="n-svn-statistics"><a href="/wiki/Statistics">Statistics</a></li>
	<li id="n-phpdoc"><a href="https://svn.wikimedia.org/doc/">Code docs</a></li>
</ul>
</div>
</div>


Using the link instead may make more sense as that should be more unique than the link text (or its message key). I can imagine a wiki having 3 sections named "Foo", "Bar" and "Baz" and all three having links like "Introduction", "Download", "Install", "Get involved", but linking to different pages.

That does indeed mean "-url" or "-link" type of message keys will end up in the ID attribute.. Or even things like "Special:Code/MediaWiki" in the case of literal page names.

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


Navigation
Links