Last modified: 2010-07-05 19:26:59 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 T26268, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 24268 - When both twinkle and mwEmbed are enabled, mwEmbed errors out.
When both twinkle and mwEmbed are enabled, mwEmbed errors out.
Status: RESOLVED FIXED
Product: mwEmbed
Classification: Unclassified
Video player (Other open bugs)
unspecified
All All
: Normal enhancement (vote)
: ---
Assigned To: Michael Dale
http://en.wikipedia.org/wiki/Wikipedi...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-05 14:09 UTC by Derk-Jan Hartman
Modified: 2010-07-05 19:26 UTC (History)
1 user (show)

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


Attachments

Description Derk-Jan Hartman 2010-07-05 14:09:31 UTC
See URL.
Comment 1 Nikola Kovacs 2010-07-05 16:54:41 UTC
The problem is this:

var moduleRequestSet = [];
...
for(var moduleName in moduleRequestSet) { ... }

This works fine as long as no-one touches Array.prototype, but Twinkle adds three new functions, which results in mwEmbed trying to load a nonexistent module called "uniq".

The solution in this case should be as simple as changing the declaration to var moduleRequestSet = {};, but that still doesn't protect against someone messing with Object.prototype, and it won't allow you to use the Array type's functions, e.g. push.

It might be a better idea to use something like Prototype's Hash.
Comment 2 Michael Dale 2010-07-05 19:26:59 UTC
Scripts should not extend base object prototypes! .. But to the extent this can be worked around I use for( i = 0; array.length; i++ ) for arrays.  If a script extends the base "Object" prototype that will be complicated to work around and slow everything down since every for loop in the entire codebase would have to check the property types instance variables and or use function heavy jQuery.each(object, function(inx, item){ /*..*/ });  iterator, which is a lot of overhead for some of loops that need to be fast. 

fixed in r69064

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


Navigation
Links