r58717 MediaWiki - Code Review archive

Why is it a tag extension, calling the inefficient recursiveTagParse()? It could just be passed through Sanitizer like any other HTML tag.

Note that you omit registering the link in $parser->mOutput, providing a vulnerability which allows spammers to evade any external link controls like the captcha trigger. In my scheme, that task, and the modification of the class attribute, would need to be done in a separate pass in internalParse().

There's also the problem of invalid HTML generation, caused by nested anchor tags. In my scheme, if the new pass were done before replaceInternalLinks(), then user-specified anchor tags could be replaced by LinkHolderArray placeholders, allowing nested links to be suppressed.

An extra pass should be quite cheap in the worst case, because most of the scanning can be done in PCRE or strcspn(). By re-entering the preprocessor with text that has already been preprocessed once, you get O(N^2) worst-case performance, and costs due to tight PHP loops and high setup time.

(Don't see a subscribe link, so replying to get follow-ups.)

<slightly tangential question to subscribe to thread>What's 'your scheme', Tim?</stqtstt>

Sanitizer would pass anchor tags through to a separate pass executed before replaceInternalLinks, which would add link placeholders for the anchor tags, register them in $parser->mOutput, and modify their attributes appropriately.

When I first wrote this, I did pass <a> just through the sanatizer. Then, while testing it, *some* really good reason come up to handle it as a magic tag. But as these things go, I can't remember what it was :)

Was it because (as you said on r58711) it didn't register an external link?

yes, right, I think that was indeed the main problem. Thanks for finding it :) I suppose to overcome this, the Sanitizer would have to have access to the ParserOutput...

Or, as I said above, you could let the Sanitizer do what it normally does, and add a separate pass for link registration to be executed before replaceInternalLinks.

Interesting topic, indeed. Anyone remembers the name of the thread at wikitech-l? I don't think it was conclusive.

(We definitely need a subscribe link)

With the debate still going on on wikitech-l, we might want to hold back this feature until we've actually decided what and how we're going to implement it.

All I see on wikitech-l is discussion about whether RDFa is suitable for Wikipedia. This feature can exist independently of the answer to that question. We can switch it off with a configuration variable if need be.

FWIW, this would be useful to microdata too, not just RDFa. Also would be nice generally, to avoid hacks like class="plainlinks" on a wrapper span.

Any move towards regularly adding raw <a> tags in wikitext is Bad. I don't see that as a hack, particularly; it's a perfectly sane and sensible use of HTML and CSS. What would be hackish is the rearguard actions we'll be fighting for months to patch up the security vulnerabilities in enabling raw <a> tags (at least with this implementation), not to mention cleaning up the blood from all the Usability people who commit suicide when they learn about the latest twist in the wikitext Gordian knot.

Oh and don't forget, what's going to happen to this post if/when raw <a> tags become whitelisted? I haven't nowiki'd them, because there's no need to; I know they're escaped by Sanitizer. How many wiki pages have made the same assumption??

• Gratuitous wrapper spans are a hack. They produce completely unnecessarily HTML.
• There should be no security vulnerabilities in adding <a>, certainly no more than style="" or whatnot. The allowed output URLs are exactly the same as for ordinary external links.
• There are no grounds for thinking usability would be hurt – these wouldn't be expected to appear in ordinary wikitext any more than, say, style="", or complex template markup. If there are common uses, we can reliably expect big wikis to wrap them in templates, like they do for everything else in existence (and adding a few more templates can't make things much worse than they are). Wikitext is so massively complicated that it's sort of silly to argue at this point that we shouldn't make it slightly less usable.
• Yes, pages that use <a> literally will break. Probably not your post, since you have no closing tag/href/etc. so it will likely be escaped anyway. This is a one-time cost that we'll see whenever we add any new tag, and is perfectly tolerable.

We enabled <span> without mass-suicide (see r6796), we should be able to do it with <a> as well.

Currently any post of <a> tag is a spambot or a blind paste. This change will make it more difficult to discern if it really was a person trying to add a link with a good idea using the "complex" syntax.

And why does external links need rel/rev? I'd expect them to be primarily used on [[internal links]], not external.

At least in some notable cases, such links would point to the canonical URL of some standardized entity, such as a creative commons license.

Yeah, there will be a bit of extra spam problems, but not enough to outweigh even a modest benefit. It's pretty easily to tell spammers anyway.

Some of these links might actually want to be internal . . . maybe this would be a good time to hack in some logic removing class="external" from external links that point to an internal domain?

Removed the tag hook for now.