Page MenuHomePhabricator

Error in MonoBook.php creates invalid XML
Closed, ResolvedPublic

Description

Author: justin.force

Description:
This is a screenshot of the error generated in Opera.

In ./skins/MonoBook.php, a space is missing which causes invalid XML to be rendered. This is a problem for strict browsers like Opera, which then fail to render the page because it is invalid XML. Here's the line:

MonoBook.php:94
<?php if($this->data['body_onload' ]) { ?>onload="<?php $this->text('body_onload') ?>"<?php } ?>

should be changed to
<?php if($this->data['body_onload' ]) { ?> onload="<?php $this->text('body_onload') ?>"<?php } ?>

which fixes the problem completely.


Version: 1.11.x
Severity: enhancement
URL: http://wiki.education.ucsb.edu

Attached:

Screenshot-Error_-_Opera.png (416×592 px, 35 KB)

Details

Reference
bz12346

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 9:57 PM
bzimport added a project: MediaWiki-Parser.
bzimport set Reference to bz12346.
bzimport added a subscriber: Unknown Object (MLST).

Can reproduce when both "section edit on right-click" and "edit on double-click" preferences are active.

Fixed in r28645

justin.force wrote:

I'm at r28671, and the bug is present. Perhaps it was re-introduced?

ayg wrote:

I can confirm the fix works correctly for me at HEAD (r28671). Could you make sure the patch was properly applied to your local copy of MW, and clarify the exact error you're getting?

justin.force wrote:

Correct the issue wherein the lack of a space in the onload attribute of the body tag in skins/MonoBook.php generates invalid XML.

The error is invalid XML rendered at line 62 of the output file which prevents rendering in the Opera web browser. The XML is invalid because there is no space preceding the 'onload' attribute of the body tag, as there should be. The error is most certainly still present in 28672, and was present in 28671 as well (just checked). I'll provide a patch, though it's just the addition of a single space character.

Attached:

robert wrote:

(In reply to comment #3)

I can confirm the fix works correctly for me at HEAD (r28671). Could you make
sure the patch was properly applied to your local copy of MW, and clarify the
exact error you're getting?

Is your server definitely serving the PHP files as XML? Normally they are configured to send it as a HTML mime because IE fails at XHTML. If it is set to serve them with an XHTML mime Opera will through errors on invalidity, otherwise AFAIK it won't.

justin.force wrote:

In my case, the server is sending Content-type: text/html headers. Opera is examining the document and determining that it is XHTML, then rejecting it as invalid.

...which is not the issue. Follow me here.

The doctype is XHTML.
XHTML is a subset of XML.
Invalid XML is being rendered.
The document is invalid.

This is a bug.

I have clearly defined it and supplied a patch. There is certainly no downside to applying my fix, and there is the practical benefit of MediaWiki rendering correct XML, being more standards-compliant, and working correctly in the very popular Opera web browser.

Come on.

The patch was already applied in r28645; most likely you've got the old generated page in your cache.