Page MenuHomePhabricator

Internal error with Modern and Monobook on specific pages
Closed, ResolvedPublic

Description

HTML of the page in URL

Example URL gives "internal error" in the top bar and all the rest of the page is eaten. Nikerabbit added some debugging output and provided the following:

[30-Oct-2013 18:18:24 UTC] PHP Warning: Illegal string offset 'id' in /www/translatewiki.net/w/includes/SkinTemplate.php

on line 1106

2013-10-30 18:18:24 v220131064414794.yourvserver.net translatewiki_net-bw_: [004b9a6b]

/wiki/Wikia:Lvs-error-already-kept-forever/es   Exception from line 1829 of 
/www/translatewiki.net/w/includes/SkinTemplate.php: Who is eating my carrots?

#0 /www/translatewiki.net/w/skins/MonoBook.php(241): BaseTemplate->makeListItem('text', 'c<<')
#1 /www/translatewiki.net/w/skins/Modern.php(77): MonoBookTemplate->cactions()
#2 /www/translatewiki.net/w/includes/SkinTemplate.php(528): ModernTemplate->execute()
#3 /www/translatewiki.net/w/includes/OutputPage.php(2078): SkinTemplate->outputPage()
#4 /www/translatewiki.net/w/includes/Wiki.php(610): OutputPage->output()
#5 /www/translatewiki.net/w/includes/Wiki.php(467): MediaWiki->main()
#6 /www/translatewiki.net/w/index.php(49): MediaWiki->run()
#7 {main}

Other pages work, for instance:
https://translatewiki.net/wiki/support?useskin=modern
http://meta.wikimedia.beta.wmflabs.org/?useskin=modern

I don't know when this started because the last line, relayed to IRC, is quite unhelpful; it might even be before
2013-04-29 23.18 -rakkaus:#mediawiki-i18n- (8 lines skipped) #7 {main}


Version: master
Severity: major
URL: https://translatewiki.net/wiki/Wikia:Lvs-error-already-kept-forever/es?useskin=modern

Attached:

Details

Reference
bz56409

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 2:25 AM
bzimport set Reference to bz56409.

The interesting part is:

BaseTemplate->makeListItem('text', 'c<<')

The second argument should be an array. The code doesn't check for that anywhere, though, and apparently something fails badly if it's not.

The two parameters are keys and values from $this->data['content_actions'] associative array, something must be messing with it in an unpleasant way.

Cursory debugging points me to the following function in Translate:

protected static function addTab( $skin, &$tabs, $name, $data, &$index ) {
    // SkinChihuahua is an exception for userbase.kde.org.
    if ( $skin instanceof SkinVector || $skin instanceof SkinChihuahua ) {
        $data['class'] = false; // These skins need it for some reason
        $tabs['namespaces'][$name] = $data;
    } else {
        array_splice( $tabs, $index++, 0, array( $name => $data ) );
    }
}

This is very messy, but my gut feeling is that it should say:

array_splice( $tabs, $index++, 0, array( array( $name => $data ) ) );

This also explains not being broken on Vector (because it's iffed out…) and on CologneBlue (because it does its own weird things with the navigation and by chance seems to ignore these entries – that might be a bug).

(That function is in TranslateEditAddons.php.)

(Moving to Translate until my gut feeling above is proven wrong.)

This is a live hack by Niklas.

siebrand@v220131064414794:/www/translatewiki.net/w$ git diff
diff --git a/includes/SkinTemplate.php b/includes/SkinTemplate.php
index e5b8872..d9b4340 100644

  • a/includes/SkinTemplate.php +++ b/includes/SkinTemplate.php @@ -1825,6 +1825,9 @@ abstract class BaseTemplate extends QuickTemplate {
    • @return string */ function makeListItem( $key, $item, $options = array() ) { + if ( !is_array( $item ) ) { + throw new MWException( "Who is eating my carrots?" ); + } if ( isset( $item['links'] ) ) { $html = ''; foreach ( $item['links'] as $linkKey => $link ) {

I don't know what he's trying to debug.

Removed some people from CC.

He's probably trying to debug this very issue. I'm almost certain that the
addTab() function splices the array wrong per comment 3. It would nice great if you could apply another live hack as described there.

Change 93176 had a related patch set uploaded by Nikerabbit:
Fix fatal error in non-Vector skins by removing nav tabs

https://gerrit.wikimedia.org/r/93176

Created attachment 13676
Empty monobook

Now most of the page content disappears in MediaWiki namespace on monobook; same thing or different?

Attached:

monobook-emptied.png (768×1 px, 17 KB)

I suppose Nike just removed the live hack. I wouldn't be surprised if the effect of a PHP fatal during skin rendering when all debug logging is turned off would be a rendered page that is simply "cut off" at one point.

Change 93176 merged by jenkins-bot:
Fix fatal error in non-Vector skins by removing nav tabs

https://gerrit.wikimedia.org/r/93176

Offending code has been removed, this should fix itself on TWN when the new version is deployed there.