Last modified: 2010-05-15 14:35:47 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 6986 - h1 #firstHeading not HTML-escaped
h1 #firstHeading not HTML-escaped
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Parser (Other open bugs)
1.7.x
All All
: Normal trivial (vote)
: ---
Assigned To: Nobody - You can work on this!
http://de.wikipedia.org/w/index.php?t...
:
: 6816 (view as bug list)
Depends on:
Blocks: html
  Show dependency treegraph
 
Reported: 2006-08-11 19:01 UTC by ASM
Modified: 2010-05-15 14:35 UTC (History)
1 user (show)

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


Attachments

Description ASM 2006-08-11 19:01:46 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).
Comment 1 Aryeh Gregor (not reading bugmail, please e-mail directly) 2006-08-11 19:08:46 UTC
(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 ***
Comment 2 ASM 2006-08-13 13:40:20 UTC
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. ***
Comment 3 Aryeh Gregor (not reading bugmail, please e-mail directly) 2006-08-13 21:15:53 UTC
(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.
Comment 4 Daniel Kinzler 2006-08-13 22:14:05 UTC
fixed in r16048
Comment 5 Aryeh Gregor (not reading bugmail, please e-mail directly) 2006-11-23 21:25:08 UTC
*** Bug 6816 has been marked as a duplicate of this bug. ***

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


Navigation
Links