Last modified: 2014-11-17 10:35: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 T35711, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 33711 - mw.util.$content is undefined in MediaWiki:Common.js
mw.util.$content is undefined in MediaWiki:Common.js
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
1.20.x
All All
: Highest normal with 1 vote (vote)
: 1.19.0 release
Assigned To: Krinkle
:
: 33669 33712 (view as bug list)
Depends on:
Blocks: 31217
  Show dependency treegraph
 
Reported: 2012-01-13 19:51 UTC by Mark A. Hershberger
Modified: 2014-11-17 10:35 UTC (History)
9 users (show)

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


Attachments
Patch for a simple implementation (3.84 KB, patch)
2012-01-24 09:41 UTC, Michael M.
Details
Sample MediaWiki:Common.js (422 bytes, text/x-js)
2012-01-24 09:42 UTC, Michael M.
Details

Description Mark A. Hershberger 2012-01-13 19:51:01 UTC
From: http://labs.wikimedia.deployment.wmflabs.org/w/index.php?diff=237

This is about http://commons.wikimedia.deployment.wmflabs.org

1.  mw.util is undefined when attempting to access from MediaWiki:Common.js without using mw.loader.load
2.   despite wrapping all inside mw.loader.using('mediawiki.util', function(){ mw.util.$content is null. But it shouldn’t since the code wrapped inside mw.loader.using('mediawiki.util'... should be executed after mw.util is initialized
Comment 1 Niklas Laxström 2012-01-13 19:53:02 UTC
Just FYI, seen on twn (more info if requested):
      2 'mw.util.$content' ist Null oder kein Objekt
      2 Uncaught exception: TypeError: Cannot convert 'mw.util' to object
     18 'util' ist Null oder kein Objekt
Comment 2 Krinkle 2012-01-14 03:31:10 UTC
(In reply to comment #0)
> From: http://labs.wikimedia.deployment.wmflabs.org/w/index.php?diff=237
> 
> This is about http://commons.wikimedia.deployment.wmflabs.org
> 
> 1.  mw.util is undefined when attempting to access from MediaWiki:Common.js
> without using mw.loader.load

I could reproduce this, but INVALID bug.

This is correct and expected behavior and has been since 1.17. Only difference is that due to the loader being improving and loading faster, race conditions are more likely to ocurr, thus requiring that dependencies are always declared. This is best done by modularizing code into gadgets. Alternatively one can use mw.loader.using inline. Previously mw.util was usually loaded before 'site'. It can happen that 'site' arrives first. Order is (and should be) asynchronous, dependency declaration is mandatory. 

> 2.   despite wrapping all inside mw.loader.using('mediawiki.util', function(){
> mw.util.$content is null. But it shouldn’t since the code wrapped inside
> mw.loader.using('mediawiki.util'... should be executed after mw.util is
> initialized

There is a difference between mw.util being undefined and mw.util.$content being null. mw.util is defined in the mediawiki.util module, which can be at any point in time. Including before the body content is parsed. For that reason mediawiki.util is and always has populated mw.util.$content from a document-ready hook. Again, due to things loading faster now (yay), it is even more important that less is assumed and more is declared.

I've reverted Rillke's changed to MediaWiki:Common.js on commons.wikimedia.beta.wmflabs back to the  original, and wrapped it in mw.loader.using.

Since mw.util.$content wasn't used there it's fine now. It is used within a $(document).ready hook but that's fine.
Comment 3 Krinkle 2012-01-14 03:34:41 UTC
*** Bug 33712 has been marked as a duplicate of this bug. ***
Comment 4 Rainer Rillke @commons.wikimedia 2012-01-14 10:38:51 UTC
mw.loader.using('mediawiki.util',function($){
console.log($)
});

$ is undefined.

So mw.util.$content can't be reached because code fails before.
Comment 5 Erwin Dokter 2012-01-14 10:41:05 UTC
*** Bug 33669 has been marked as a duplicate of this bug. ***
Comment 6 Rainer Rillke @commons.wikimedia 2012-01-14 10:45:31 UTC
just for convenience http://commons.wikimedia.beta.wmflabs.org/wiki/MediaWiki:Common.js
Comment 7 Rainer Rillke @commons.wikimedia 2012-01-16 17:49:44 UTC
If noone cares, I have to reopen this bug.
1) Dependencies. Ok. Just wanted a confirmation. Please note that also the user's common.js / vector.js are affected. This will cause a lot of trouble since MediaWiki:Common.js waits for mw.util while user's js is executed. This will cause all "importScript"s and other calls to be rewritten by each user. Or we invent something to queue the import-requests and execute them when ready.

2) mw.util.$content is still null in http://commons.wikimedia.beta.wmflabs.org/wiki/MediaWiki:Common.js *despite being wrapped inside a $(function() {})*. I added debug code and uploaded a screenshot: http://commons.wikimedia.beta.wmflabs.org/wiki/MediaWiki:Common.js
Comment 8 Erwin Dokter 2012-01-19 12:44:57 UTC
(In reply to comment #4)
> mw.loader.using('mediawiki.util',function($){
> 
> $ is undefined.
> 
> So mw.util.$content can't be reached because code fails before.

Your logic fails... $ and $content are unrelated. $ is an alias for jQuery, while $content is an alias for #bodyContent (depending on skin). function($) is pointless, as the callback from mw.loader.using() does not pass anything (which is fortunate, as that would break jQuery).
Comment 9 Erwin Dokter 2012-01-19 18:07:17 UTC
Something seems not to be working quite right. Below are two queries for both mw.util.$content and $('#bodyContent'), both in Monobook and Vector.

querie format:
console.log( 'Begin of Common.js: mw.util.$content is', typeof( mw.util.$content ), mw.util.$content );
console.log( 'Begin of Common.js: #bodyContent is', typeof( $( '#bodyContent' ) ), $( '#bodyContent' ) );

Result in Monobook:
[18:59:47.760] Begin of Common.js: mw.util.$content is object null
[18:59:47.766] Begin of Common.js: #bodyContent is object ({length:1, 0:({}), context:({}), selector:"#bodyContent"})
[18:59:47.777] End of Common.js: mw.util.$content is object null
[18:59:47.783] End of Common.js: #bodyContent is object ({length:1, 0:({}), context:({}), selector:"#bodyContent"})
[18:59:47.789] On document.ready: mw.util.$content is object null
[18:59:47.796] On document.ready: #bodyContent is object ({length:1, 0:({}), context:({}), selector:"#bodyContent"})

Result in Vector:
[19:00:35.994] Begin of Common.js: mw.util.$content is object null
[19:00:36.001] Begin of Common.js: #bodyContent is object ({length:1, 0:({}), context:({}), selector:"#bodyContent"})
[19:00:36.012] End of Common.js: mw.util.$content is object null
[19:00:36.017] End of Common.js: #bodyContent is object ({length:1, 0:({}), context:({}), selector:"#bodyContent"})
[19:00:36.024] On document.ready: mw.util.$content is object null
[19:00:36.031] On document.ready: #bodyContent is object ({length:1, 0:({}), context:({}), selector:"#bodyContent"})
Comment 10 Rainer Rillke @commons.wikimedia 2012-01-19 21:12:17 UTC
(In reply to comment #8)
> Your logic fails... $ and $content are unrelated.

I suggest you read the changes Krinkle made. I know that they are not the same.

But you should also read what Krinkle wrote here:
> Since mw.util.$content wasn't used there it's fine now. It is used within a
$(document).ready hook but that's fine.
Krinkle thinks that when adding a dependency to mw.util AND wrapping the code accessing mw.util.$content inside a $(document).ready(function(){...}) or its short form $(function(){...}), mw.util.$content must be defined, which is not the case here why I reopened the bug.

http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/resources/mediawiki/mediawiki.util.js?view=markup#l58
If you look at the source-code of the mw.util -module you will see that $content is predefined with null and there is a "init"-method that sets $content to what it should be.

The problem? is that init can only be invoked by $(document).ready() because RL-modules may be loaded before the DOM is ready.

So the question to solve is, why mw.util.$content is null.

> I've reverted Rillke's changed to MediaWiki:Common.js on
commons.wikimedia.beta.wmflabs back to the  original, and wrapped it in
mw.loader.using.
http://commons.wikimedia.beta.wmflabs.org/w/index.php?title=MediaWiki%3ACommon.js&diff=22461&oldid=22442
sans commentaire
Comment 11 Erwin Dokter 2012-01-19 23:23:45 UTC
Also interesting:

/**
 * Initialisation
 * (don't call before document ready)
 */
init: function() { ...

I don't *when* RL calls init, but is it is before document.ready, perhaps we will have to wrap common.js itself in $(document).ready, then load util within.
Comment 12 Erwin Dokter 2012-01-20 00:31:48 UTC
Wrapping mw.loader.using() itself in a $(document).ready works:

Begin of Common.js: mw.util.$content is object [<div id=​"bodyContent">​...</div>]
Begin of Common.js: #bodyContent is object [<div id=​"bodyContent">​...</div>]

How does ResourceLoader ensures document.ready before loading mw.util? If it doesn't, then $content will never be properly defined.
Comment 13 Michael M. 2012-01-20 11:43:28 UTC
mw.util.$content is initialized by mediawiki.page.ready, so if you need mw.util.$content, you should register $(document).ready after mediawiki.page.ready does so.
So I think it will work if you wrap the whole code into
mw.loader.using(['mediawiki.util', 'mediawiki.page.ready'], function () {

});

(which I didn't try and which seems ugly to me).
Comment 14 Krinkle 2012-01-20 11:44:30 UTC
document-ready is a jQuery event (queue). Both your handler and the mw.util.init are added to that queue. Depending on in which order jQuery executes them, it will mean that for your script it's undefined or not.

I remember a bug about this a few months back and we fixed (it was a bug about the unit test being unable to test the initializer becuase it initilized upon loading the script). As a good practice its recommended to separate the definitions of methods and actually executing code, and so we (I) did.

As a result the mediawiki.page.ready module was added, which would actually initilize stuff (i.e. one should be able to load jquery.checkboxShiftClick without having it do something by default that can't be undone).

However this doesn't change the fact that document-ready is still an event queue of which the order is practically uncontrollable.

Wrapping the mw.loader.using call inside document-ready instead of the other way around doens't sound like something that should work or is supported so I don't recommend relying on that. It actually seems even counter-intuitive as it would add it to the queue before loading mw.util which would enqueue itself after yours.

Anyway I remember a fix for this, but I can't think of what it was. I'll take a fresh look at this tonight.
Comment 15 Krinkle 2012-01-20 11:46:48 UTC
(mid-air collision)

(In reply to comment #10)
> (In reply to comment #8)
> > Since mw.util.$content wasn't used there it's fine now. It is used within a
> $(document).ready hook but that's fine.
> Krinkle thinks that when adding a dependency to mw.util AND wrapping the code
> accessing mw.util.$content inside a $(document).ready(function(){...}) or its
> short form $(function(){...}), mw.util.$content must be defined, which is not
> the case here why I reopened the bug.
> 

That is indeed what I thought.
Having mw.util available is good, but $content isn't available until document-ready. init() is being ccalled in document-ready, becuase before that event there is nothing $content can be set to, the elements need to exist before they can be selected and cached in $content.
Comment 16 Erwin Dokter 2012-01-20 14:07:41 UTC
Why not make $content a function queued in document.ready? That should ensure it always returns populated.
Comment 17 Michael M. 2012-01-23 08:19:18 UTC
I think I found a good solution for the problem. I didn't test it, but it should work and additionally solve several other bugs (bug 30713, bug 33399):

1. In mediawiki.util.js create a jQuery.Callback( 'memory' ) and make it a public member of mw.util, perhaps as mw.util.callback
2. In mediawiki.page.ready.js call mw.util.callback.fire() for it after mw.util.$content has been initialized.
3. Whenever the content is changed (at least Live Preview does this) call mw.util.callback.fire() again.
4. Replace $( document ).ready( callback ) with mw.util.callback.add( callback ) everywhere where the callback relies on $content and should be called when the content changes (in mediawiki.page.ready.js at least makeCollapsible and tablesorter).
Comment 18 Rob Lanphier 2012-01-23 22:45:48 UTC
Is this a duplicate of bug 33746?
Comment 19 Krinkle 2012-01-23 22:51:58 UTC
(In reply to comment #18)
> Is this a duplicate of bug 33746?

No, the users reporting this bug do have the module loaded. But a certain variable isn't initialized when accessed before the document is ready.

(In reply to comment #17)
> I think I found a good solution for the problem. I didn't test it, but it
> should work and additionally solve several other bugs (bug 30713, bug 33399):
> 
> 1. In mediawiki.util.js create a jQuery.Callback( 'memory' ) a [...]

The general concept of these kind of hooks is good, but I'd rather not do it ad hoc inside mw.util. There is is a general bug about the need for this, among the reasons is the ability to have ajax preview represent a normal view by having javascript handlers run on a 'mw.wikipage.onload' hook instead of window.document.ondomready. This is requested in bug 23580 and in bug 23580 comment 13 I had the same idea as you, using jQuery.Callbacks indeed.
Comment 20 Michael M. 2012-01-24 09:41:09 UTC
Created attachment 9900 [details]
Patch for a simple implementation

The patch just does what I suggested in comment 17. Please note that I ran into huge troubles with the edit toolbar, I opened bug 33922 for this. Feel free to do whatever you think should be done with the patch.
Comment 21 Michael M. 2012-01-24 09:42:05 UTC
Created attachment 9901 [details]
Sample MediaWiki:Common.js

Example MediaWiki:Common.js to show that/how it works.
Comment 22 Krinkle 2012-02-02 00:48:48 UTC
Resolved in r110542.

Advanced callback system will also soon be done by bug 23580.
Comment 23 Erwin Dokter 2012-02-02 12:27:20 UTC
Now that mw.util is loaded at the top as a dependency of mw.page.startup, does this not in fact negate bug 33746/r110254 ?

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


Navigation
Links