Last modified: 2010-05-15 14:35:47 UTC
When editing an article with a title containing special characters, such as the &, the heading in the h1#firstHeading-tag is not being escaped. Example: http://de.wikipedia.org/w/index.php?title=AT%26T&action=edit According to Duesentrieb, this bug does not occur in the Cologne Blue skin but in all other skins (tested on Monobook, Simple, Myskin).
(In reply to comment #0) > According to Duesentrieb, this bug does not occur in the Cologne Blue skin but > in all other skins (tested on Monobook, Simple, Myskin). It's a Monobook issue. Simple and Myskin are Monobook-based, Cologne Blue is Standard-based. *** This bug has been marked as a duplicate of 3243 ***
Since I am new to this, I want to apologize in advance in case I am infringing any policies here, but nevertheless I would like to state what I (think to) have found out about this bug. AFAICS, the bug is caused by line 96 of MonoBook.php: <?php $this->data['displaytitle']!=""?$this->text('title'):$this->html('title') ?> The displaytitle-if-condition was initially added in revision 13572. Previously, the code was only $this->text('title'). These two member functions only differ by the htmlspecialchars function, which is applied in text() but not in html(). After searching the whole source code, it seems to me as if the value of displaytitle is never assigned (only by a hook, which is disabled by default by $wgAllowDisplayTitle), therefor always an empty string. That means, logically, that in the MonoBook-skin, titles always lack html escaping. Regarding all these facts, my conclusion would be just to change the above (current) code back to the previous one. As I initially said, I'm sorry for posting this in case you don't want any code analyses here, otherwise I would appreciate any comments on this. *** I have reopened the bug because I don't think it is helpful to mark it as a duplicate of a bug which is more than one year old and not yet solved. ***
(In reply to comment #2) > The displaytitle-if-condition was initially added in revision 13572. Previously, > the code was only $this->text('title'). These two member functions only differ > by the htmlspecialchars function, which is applied in text() but not in html(). > After searching the whole source code, it seems to me as if the value of > displaytitle is never assigned (only by a hook, which is disabled by default by > $wgAllowDisplayTitle), therefor always an empty string. That means, logically, > that in the MonoBook-skin, titles always lack html escaping. Regarding all these > facts, my conclusion would be just to change the above (current) code back to > the previous one. Upon some poking, I agree with all but your suggested fix. The correct fix is to swap the text and HTML functions: -<?php $this->data['displaytitle']!=""?$this->text('title'):$this->html('title') ?> +<?php $this->data['displaytitle']!=""?$this->html('title'):$this->text('title') ?> This means that if a hook *does* want to change the displayed title somehow, it will be able to feed HTML tags to the skin, after escaping the title itself. Not that, as you point out, any official extension currently appears to use this. I assume this was the intent of r13572. > As I initially said, I'm sorry for posting this in case you don't want any code > analyses here, otherwise I would appreciate any comments on this. Code analyses do belong here, yep. Submitted patches are also good. > *** I have reopened the bug because I don't think it is helpful to mark it as a > duplicate of a bug which is more than one year old and not yet solved. *** Well, no, the whole point is to coordinate discussion. However, I erred in duping it to bug 3243, because 3243 is much broader (probably too broad to be a single bug, really). I could've sworn I saw this bug before, but whatever.
fixed in r16048
*** Bug 6816 has been marked as a duplicate of this bug. ***
