r52963 MediaWiki - Code Review archive

Some background to this on bug 19576.

This should probably get a RELEASE-NOTES mention.

Done in r53101.

This adds two id's to the h# element, if the header has a manually specified id. Like <h2 id=test>Foo</h2> becomes <h2 id="Foo" id="test">Foo</h2>. This needs to be dealt with somehow. Note that overriding the given id will break things: for instance, w:Main Page uses explicit id's on anchors.

Can we simply scan $attribs for an existing id attribute and avoid the default $anchor-based id if there is?

What do you mean by avoiding it? The id needs to be somewhere. We could put it on the div instead of the actual header element, perhaps.

I was thinking that if they'd specified an id for the header, they probably didn't need the default id provided by the text - but I guess it could break things that expected them both to work.

Modifying the div seems a little hacky - what if something put an id on that? Maybe we could just fall back to the old <a id=\"$anchor\"></a>" for the situation where a custom id was detected in $attribs.

As long as the <h2> syntax creates an entry in the table of contents, it needs an ID (preferably attached directly to the h2 tag). For consistency, IDs should match the section titles. For example, <h2>Foo</h2> should become <h2 id="Foo">Foo</h2>. If editors want to be able to link to headers by other names, they can simply add span or anchor tags. For example, if the section "Foo" should be linkable by #Bar, the editor might write <span id="Bar"></span><h2>Foo</h2>. Templates can make this as easy as {{anchor|Bar}}<h2>Foo</h2>.

So, I recommend that we simply do not allow editors to add IDs directly to header tags. Editors that need unusual IDs for URL purposes can add them pretty easily with span or anchor tags.

What's wrong with adding the TOC id to the div instead of the h# tag? That has the advantage of not breaking existing content.

All right, just so we're all clear, <h2>Foo</h2> produces

<h2 id="Foo"> <span class="mw-headline">Foo</span></h2>

The only problem I see with moving id="Foo" to <span class="mw-headline"> is that it would decrease the ID's usefulness a little. Without the editor adding more code, CSS would only be able to target the headline and not the header as a whole. Similar problems would crop up in JavaScript as well.

So, in the interest of flexibility, I think I'd rather keep it how it is, minus the ability for editors to add custom IDs to headers. Or maybe we could get the TOC to use the custom ID (if present) instead of the autogenerated one.

Actually, my favorite solution would be to have <h2>Foo</h2> output <h2>Foo</h2>, plain and simple. No edit link, no automatic ID, no entry in the table of contents.

But, since that would require a lot more work on the parser than I feel like doing, let's just go with your solution for now. Committed in r53577.

I always thought that too, but it would break compatibility at this point somewhat severely.

You can't put an id on the div. There's no syntax for it, since the div is generated by the software.

AFAIK outstanding problems have been resolved in later commits.