Last modified: 2012-12-12 02:08:09 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 T31106, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 29106 - Clean up use of JavaScript legacy globals and other code quality issues
Clean up use of JavaScript legacy globals and other code quality issues
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
WikiEditor (Other open bugs)
unspecified
All All
: Normal minor (vote)
: ---
Assigned To: Krinkle
: patch, patch-reviewed
: 31102 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-23 07:20 UTC by Michael M.
Modified: 2012-12-12 02:08 UTC (History)
6 users (show)

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


Attachments
Patch for wg.* -> mw.config.get (10.16 KB, patch)
2011-06-25 07:37 UTC, Michael M.
Details
Proposed patch (10.44 KB, patch)
2011-07-02 08:05 UTC, Michael M.
Details

Description Michael M. 2011-05-23 07:20:43 UTC
Legacy globals:
 wgArticlePath in jquery.wikiEditor.dialogs.config.js
 wgExtensionAssetsPath in jquery.wikiEditor.iframe.js and jquery.wikiEditor.js
 wgPageName in jquery.wikiEditor.previewDialog.js and jquery.wikiEditor.toc.js
 wgScriptPath in jquery.wikiEditor.dialogs.config.js (twice) and jquery.wikiEditor.previewDialog.js
 wgServer in jquery.wikiEditor.dialogs.config.js
 wgTitle in jquery.wikiEditor.previewDialog.js
 wgUrlProtocols in jquery.wikiEditor.dialogs.config.js
 wgUserLanguage in jquery.wikiEditor.js (twice)

$j in:
 ext.wikiEditor.preview.js
 ext.wikiEditor.publish.js
 jquery.wikiEditor.dialogs.js
 jquery.wikiEditor.js
 jquery.wikiEditor.previewDialog.js
 jquery.wikiEditor.publish.js

Accidently global variables
Firebug lists some variables in global scope which probably come from WikiEditor and are meant to be local:
 $group
 cell
 filter
 group
 heading
 option
 page
 row
 section
 tool
 type

For-in-loops:
 Some for-in-loops (e. g. for ( filter in tool.filters ) in jquery.wikiEditor.toolbar.js) seem to run not over an object but over an array. Use the for (;;) form instead.
Comment 1 Roan Kattouw 2011-06-03 09:53:46 UTC
Krinkle, could you clean these up?
Comment 3 Krinkle 2011-06-06 23:19:05 UTC
A good start was done in r89616.
Comment 4 Michael M. 2011-06-25 07:37:28 UTC
Created attachment 8702 [details]
Patch for wg.* -> mw.config.get

I've added a patch to use mw.config.get for all global variables. This also uses mw.util.wikiScript( 'api' ) for the path to api.php.
Comment 5 Michael M. 2011-06-25 07:42:01 UTC
I just missed one global variable:
In WikiEditor/modules/jquery.wikiEditor.toolbar.config.js line 939
change stylepath to mw.config.get( 'stylepath' )
Comment 6 Krinkle 2011-06-26 20:09:18 UTC
Applied by mah in r90845.

@Michael: What're we at ? Can we mark this bug fixed ?
Comment 7 Michael M. 2011-06-28 07:39:35 UTC
There is still the problem with the for-in-loops: Set something like
Array.prototype.random = 'stuff';
in startup.js to break everything that can be broken and try to get
ext.wikiEditor.tests.toolbar
to work again.
Comment 8 Michael M. 2011-07-02 08:05:25 UTC
Created attachment 8732 [details]
Proposed patch

I broke everything with Array.prototype.random = 'stuff'; and fixed it again, patch added.

Please note the following:

* Adding something to the prototype for Array broke jquery.client as well, I opened bug 29676 with a patch for this.
* Some changes in jquery.wikiEditor.toolbar.js are unrelated to this, I added quotes for the [rel=] stuff, since this broke one test. Some other changes were done to satisfy JSHint.
* The changes in jquery.wikiEditor.preview.js are definitly unrelated to this, but I didn't want to create a separate patch for just adding a 2 (otherwise the diff is shown to the whole text, even when editing only a section) and for exchanging the + with a , (which just didn't work, asking for api.php[object Object] certainly gives a 404 error).
* I didn't care about jquery.highlight.js, if a parameter isn't documented one can't know if it's an array or a plain object.
Comment 9 Derk-Jan Hartman 2011-07-02 09:17:50 UTC
Split up the patch myself. Committed in.

r91342
r91343
r91347
r91349
Comment 10 Brion Vibber 2011-09-22 23:40:47 UTC
*** Bug 31102 has been marked as a duplicate of this bug. ***

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


Navigation
Links