Last modified: 2013-11-03 11:28:03 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T58409, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 56409 - Internal error with Modern and Monobook on specific pages
Internal error with Modern and Monobook on specific pages
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
Translate (Other open bugs)
master
All All
: Unprioritized major (vote)
: ---
Assigned To: Niklas Laxström
https://translatewiki.net/wiki/Wikia:...
:
Depends on:
Blocks: 39480
  Show dependency treegraph
 
Reported: 2013-10-31 07:30 UTC by Nemo
Modified: 2013-11-03 11:28 UTC (History)
4 users (show)

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


Attachments
HTML of the page in URL (2.58 KB, text/plain)
2013-10-31 07:30 UTC, Nemo
Details
Empty monobook (17.47 KB, image/png)
2013-11-02 21:05 UTC, Nemo
Details

Description Nemo 2013-10-31 07:30:12 UTC
Created attachment 13636 [details]
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}
Comment 1 Bartosz Dziewoński 2013-10-31 10:07:50 UTC
Also affects Monobook and other subpages of that page, e.g. https://translatewiki.net/wiki/Wikia:Lvs-error-already-kept-forever?useskin=monobook
Comment 2 Bartosz Dziewoński 2013-10-31 10:13:45 UTC
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.
Comment 3 Bartosz Dziewoński 2013-10-31 10:34:26 UTC
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).
Comment 4 Bartosz Dziewoński 2013-10-31 10:35:32 UTC
(That function is in TranslateEditAddons.php.)
Comment 5 Bartosz Dziewoński 2013-10-31 10:39:14 UTC
(Moving to Translate until my gut feeling above is proven wrong.)
Comment 6 Siebrand Mazeland 2013-10-31 11:32:53 UTC
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.
Comment 7 Bartosz Dziewoński 2013-10-31 12:13:07 UTC
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.
Comment 8 Gerrit Notification Bot 2013-11-02 19:28:04 UTC
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
Comment 9 Nemo 2013-11-02 21:05:59 UTC
Created attachment 13676 [details]
Empty monobook

Now most of the page content disappears in MediaWiki namespace on monobook; same thing or different?
Comment 10 Bartosz Dziewoński 2013-11-02 21:09:19 UTC
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.
Comment 11 Gerrit Notification Bot 2013-11-03 10:06:46 UTC
Change 93176 merged by jenkins-bot:
Fix fatal error in non-Vector skins by removing nav tabs

https://gerrit.wikimedia.org/r/93176
Comment 12 Bartosz Dziewoński 2013-11-03 11:28:03 UTC
Offending code has been removed, this should fix itself on TWN when the new version is deployed there.

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


Navigation
Links