Last modified: 2014-11-17 10:36:28 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 T32713, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 30713 - Add mw.hook event for wikipage content ready
Add mw.hook event for wikipage content ready
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
unspecified
All All
: High enhancement with 5 votes (vote)
: 1.21.0 release
Assigned To: Krinkle
:
Depends on: 23580
Blocks: 36060 51028 51029 69108 33399 51565 55893
  Show dependency treegraph
 
Reported: 2011-09-02 21:58 UTC by Krinkle
Modified: 2014-11-17 10:36 UTC (History)
14 users (show)

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


Attachments

Description Krinkle 2011-09-02 21:58:57 UTC
Stuff that is currently in page.ready is whatever should run when the document is ready.

However a long-time known issue is that these are not re-run when an article is rendered after the document-ready event (for example when using ajax/live preview).

As a result collapsible elements, sortable tables and what not don't work when previewing articles.

By having these custom MediaWiki event based, they will run whenever needed, even multiple times during a page's live.

Other events may be the view of a diff. Several gadgets to ajax-patrolling that show many diffs on the page over time. Other gadgets that enhance a diff view (eg. gadgets that annotate the diff itself, or gadgets like Twinkle that add toollinks for the user names on both sides of the diff).
Comment 1 Krinkle 2012-01-20 23:20:47 UTC
We need this soon, but probably best to wait a little longer until 1.19 is out of the way (feature-frozen)
Comment 3 joan.creus.c 2012-04-08 11:43:44 UTC
Here's some test code which would accomplish what was described above. Probably it needs many changes, it already received some changes via IRC. There would be events for various page actions. Some of them could be: pageready, article.load, history.load, edit.load, special.load. 'pageready' would be equivalent to $(document).ready (so probably it's not necessary). The others would fire on load, but also other scripts could fire them whenever they need it (for example, a script which loads into mw.util.$content pages).

It would require to many changes internally, I tested some of the changes and it does work. Many scripts which use $(document).load should be changed appropiately (some needn't). Also, scripts on Wikipedias would have to adapt. Overall, maybe too drastic changes, but the benefeits would also be great:
- Wikis could be ajaxified completely, making pages much faster. I'm already working on this, and I found basically that problem.
- When Wikipedias have to change, everything would keep working as usual.

There aren't many lines so I'm posting it inline.

		hooks: ( function () {
			var callbacks = {};

			function getCb( key ) {
				if ( callbacks[key] === undefined ) {
					callbacks[key] = $.Callbacks( "memory" );
				}
				return callbacks[key];
			}

			function getArray( obj ) {
				return Array.prototype.slice.call(obj);
			}

			return {
				add: function (hook) {
					getCb(hook).add( getArray ( arguments ).splice(1) );
				},
				run: function (hook) {
					getCb(hook).fire.apply( window, getArray( arguments ).splice(1) );
				}
			};
		}() )

It would be part of the mediawiki object.
Comment 4 joan.creus.c 2012-04-09 08:56:00 UTC
Here's some fixed code. It includes a "remove" function, also, and functions return 'this' so that they can be chained (behaving the same way as jQuery Callbacks).

		hooks: ( function () {
			var callbacks = {};

			function getCb( key ) {
				if ( callbacks[key] === undefined ) {
					callbacks[key] = $.Callbacks( "memory" );
				}
				return callbacks[key];
			}

			return {
				add: function (hook) {
					var callback = getCb(hook);
					callback.add.apply( callback, Array.prototype.splice.call( arguments, 1 ) );
					return this;
				},
				remove: function (hook) {
					var callback = getCb(hook);
					callback.remove.apply( callback, Array.prototype.splice.call( arguments, 1 ) );
					return this;
				},
				run: function (hook) {
					var callback = getCb(hook);
					callback.fire.apply( callback, Array.prototype.splice.call( arguments, 1 ) );
					return this;
				}
			};
		}() ),
Comment 5 Krinkle 2012-04-09 20:56:07 UTC
Hi Joan,

That looks pretty good, a few small points:
* One of the things that makes jQuery.Callbacks great is it's context independence (allowing the methods to be detached and still be working properly when called). It is great that this latest patch has the chainability (which the previous patch didn't), however if you're going for that, make it also context independent so that users can expect similar features with detachment that jQuery.Callbacks has. You can do this by storing the hooks methods object in a 'hooks' variable and using the var 'hooks' instead of 'this', and then return hooks instead of {} or this.
* "getCb Init" in remove() seems unneeded since an inexistant callback object can't have anything
* A little spacing in the "function ( hook )" definitions and the calls to "getCb( hook )"

When you fix those points I think this is ready for experimentation. Then comes the second big step and that is the implementation of the "run" calls in MediaWiki core (the third step of implementing the "add" calls should be saved for last), and the third step of documenting those (just like we document the php hooks in ./docs/hooks.txt and [[mw:Manual:Hooks]]).
Comment 6 Daniel Friesen 2012-08-13 07:59:21 UTC
I won't disagree that a hook system would be interesting to have client-side. But for the case of replacing dom-ready we are really dealing with DOM elements and we should fire a dom event on the root where we inserted content instead of firing some global hook.

For that purpose I introduced a mw-content-ready event in Gerrit change #19204 that triggers mw-content-ready on body on DOM ready and on #wikiPreview when live-preview is used.
Comment 7 Daniel Friesen 2012-08-13 09:33:53 UTC
This has some overlap with bug 23580.

This bug should probably focus on the hook/event that happens on domready/preview and whether it should be a hook or a dom event. While the other bug focuses on the introduction of a mw.hooks for anything we need it for.
Comment 8 Krinkle 2012-08-16 12:31:11 UTC
(In reply to comment #7)
> This has some overlap with bug 23580.
> 
> This bug should probably focus on the hook/event that happens on
> domready/preview and whether it should be a hook or a dom event. While the
> other bug focuses on the introduction of a mw.hooks for anything we need it
> for.

I agree. I've renamed this bug to implement a hook for "article ready" (which will be executed with <div id="bodyContent"></div> once by default, and can be re-triggered by other scripts when and as appropiate.

Made bug 23580 a dependency. As we'll need
> mw.hooks.run( 'article.load', .. );

.. in the page output in order for
> mw.hooks.add( 'article.load', function ( $content () {
>     ...;
> } );
 to work.
Comment 9 Daniel Friesen 2012-08-16 13:30:27 UTC
Btw, page load and preview load aren't the only situation where we want to fire this event.

For example, say an extension implements something like tabs that loads contents of other tabs or whatnot using ajax. We want the extension to fire this on the dom node that was just inserted in the page so that we get things like collapsibility in that area.

That's why I went with 'content' rather than 'article' since this really shouldn't be only when a full article is loaded, but when any new user content shows up in the dom.
Comment 10 Mark A. Hershberger 2012-09-28 21:27:58 UTC
Unless someone wants to do this in the next week or so, pushing.
Comment 11 Krinkle 2013-03-31 00:48:44 UTC
Change-Id: Ic73a3efe53d6fb731e7f1e531d5f51530cd7e4fe
Comment 12 Krinkle 2013-05-22 23:11:44 UTC
(In reply to comment #11)
> Change-Id: Ic73a3efe53d6fb731e7f1e531d5f51530cd7e4fe

Merged.
Comment 13 Rainer Rillke @commons.wikimedia 2013-07-09 11:49:05 UTC
Just for the record (because no one wrote it here):

https://git.wikimedia.org/blob/mediawiki%2Fcore/ffc14761a50740a6ffbdf39e6a5c5a85b51713cc/resources%2Fmediawiki.action%2Fmediawiki.action.edit.preview.js#L100

$( mw ).trigger( 'LivePreviewDone', [copySelectors] );

^^ This is how the event is currently emitted for LivePreview.
Comment 14 Rainer Rillke @commons.wikimedia 2013-07-09 11:57:01 UTC
Please rethink Daniel Friesen's Gerrit change #19204 for consistency. If you insert DOM nodes and then trigger the event on mw, this seems to be stupid to me. I am also unable to see a hook here.
Comment 15 Krinkle 2013-07-09 21:12:38 UTC
(In reply to comment #13)
> Just for the record (because no one wrote it here):
> 
> https://git.wikimedia.org/blob/mediawiki%2Fcore/
> ffc14761a50740a6ffbdf39e6a5c5a85b51713cc/resources%2Fmediawiki.
> action%2Fmediawiki.action.edit.preview.js#L100
> 
> $( mw ).trigger( 'LivePreviewDone', [copySelectors] );
> 
> ^^ This is how the event is currently emitted for LivePreview.

That's not how it is supposed to be used though. I introduced the wikipage.content event in mw.hook for core, but non-core isn't using this yet (by core I mean the initial page load from the server side).

live preview should emit that event instead so that tools (e.g. jquery.tablesorter, jquery.makeCollapsible etc.) can reliably bind their init to that instead of doing both document-ready and the non-standard jQuery() event wrapping on the "mw" object.
Comment 16 Gerrit Notification Bot 2013-07-18 01:39:24 UTC
Change 74312 had a related patch set uploaded by Krinkle:
Move firing of "wikipage.content" mw.hook out of mediawiki.util

https://gerrit.wikimedia.org/r/74312
Comment 17 Gerrit Notification Bot 2013-07-18 01:39:34 UTC
Change 74313 had a related patch set uploaded by Krinkle:
mediawiki.page.ready: Use wikipage.content instead of domready

https://gerrit.wikimedia.org/r/74313
Comment 18 Gerrit Notification Bot 2013-07-24 12:22:17 UTC
Change 74312 merged by jenkins-bot:
Move firing of "wikipage.content" mw.hook out of mediawiki.util

https://gerrit.wikimedia.org/r/74312
Comment 19 Gerrit Notification Bot 2013-07-26 00:18:12 UTC
Change 74313 merged by jenkins-bot:
mediawiki.page.ready: Use wikipage.content instead of domready

https://gerrit.wikimedia.org/r/74313
Comment 20 Matthew Flaschen 2014-08-04 21:07:31 UTC
This doesn't depend on bug 69108.  I think you meant that depends on this.

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


Navigation
Links