Last modified: 2011-11-23 13:30:10 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 T31569, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 29569 - Calling $out->addModuleStyles( 'invalid.module' ); with an invalid module name causes php fatal
Calling $out->addModuleStyles( 'invalid.module' ); with an invalid module nam...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
1.20.x
All All
: Normal normal (vote)
: ---
Assigned To: Trevor Parscal
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-24 19:21 UTC by Anton Kochkov
Modified: 2011-11-23 13:30 UTC (History)
4 users (show)

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


Attachments

Description Anton Kochkov 2011-06-24 19:21:17 UTC
1st report
----------

I was working on skinning my wiki, and was going to make a custom skin based on Vector. I made a duplicate of the vector folder and both PHP files, with a new name (zelda). Then, I replaced all mentions of Vector with Zelda and vector with zelda. I am currently getting an error, though, which I am baffled by.

Fatal error: Call to a member function getGroup() on a non-object in /home/pidginet/public_html/test/includes/OutputPage.php on line 2707

Line 2707 is:
$group = $resourceLoader->getModule( $name )->getGroup();

MediaWiki 1.17.0beta1
 PHP 5.2.9 (cgi-fcgi)
 MySQL 5.0.92-community
 Apache 2.2.16

2nd report
----------

Same error on trunk of MediaWiki

changed all with s/vector/droiddev/ s/Vector/DroidDev/

but line in OutputPage.php is 2995

MediaWiki 1.19alpha (r90677)
PHP 5.3.6-pl1-gentoo (fpm-fcgi)
PostgreSQL 9.0.4
Comment 1 Bawolff (Brian Wolff) 2011-06-24 19:26:47 UTC
Changing every instance of vector to something else might not be a good idea, because not every instance of the word vector may correspond to the skin name.

It sounds like you changed a reference of which module to load, without making a 'zelda' module (That's a total guess though)

I'm leaning towards this is not a bug.
Comment 2 Anton Kochkov 2011-06-24 19:45:24 UTC
only change words vector in css classnames, as skin name and in various var names. and changed module/skin name to new word, also renamed directory with css and images.
Comment 3 Bawolff (Brian Wolff) 2011-06-24 22:44:45 UTC
The line you changed that broke everything is:               $out->addModuleStyles( 'skins.vector' );


(In reply to comment #2)
> only change words vector in css classnames, as skin name and in various var
> names. and changed module/skin name to new word, also renamed directory with
> css and images.

Well yes, doing a global find and replace on part of the codebase is a good way to introduce bugs. Anyhow that's kind of off topic since bugzilla is not a support forum.

-----
Re-purposing bug:
Expected behaviour:
*Calling $out->addModuleStyles( 'some.invalid.module' ); should either ignore the invalid module, or throw an exception.

Actual behaviour:
Fatal error: Call to a member function getGroup() on a non-object in
/home/pidginet/public_html/test/includes/OutputPage.php on line 2707

I'm not sure which of ignoring or throwing an exception is better. I personally prefer throwing an exception, however (based on a quick look) scriptModules that are invalid already seem to be ignored so there is precedent that way (but this ignoring seems to be a side effect of filtering them for top/bottom, and not intentional)
Comment 4 Daniel Friesen 2011-11-21 02:22:18 UTC
At the least we could use wfDebug or wfWarn.

Taking a look at OutputPage.php right now we still use $module = $resourceLoader->getModule( $name ); and then right after start maing calls to $module. ResourceLoader::getModule explicitly says that the return value of getModule may be null, so this doesn't look like proper coding. That portion of OutputPage needs to properly guard against null module returns (just as we guard against null titles) and if it does any failing at all it should at the very least do so using a descriptive message about how a module does not exist, instead of throwing cryptic error messages as a result of trying to call something on null.
Comment 5 Roan Kattouw 2011-11-23 13:30:10 UTC
r104030 fixes the OutputPage issue described in comment 3 and comment 4.

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


Navigation
Links