Add mw.hook event for wikipage content ready
Closed, ResolvedPublic

Description

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).


Version: unspecified
Severity: enhancement

bzimport set Reference to bz30713.
Krinkle created this task.Via LegacySep 2 2011, 9:58 PM
Krinkle added a comment.Via ConduitJan 20 2012, 11:20 PM

We need this soon, but probably best to wait a little longer until 1.19 is out of the way (feature-frozen)

bzimport added a comment.Via ConduitApr 8 2012, 11:43 AM

joan.creus.c wrote:

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.

bzimport added a comment.Via ConduitApr 9 2012, 8:56 AM

joan.creus.c wrote:

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;
				}
			};
		}() ),
Krinkle added a comment.Via ConduitApr 9 2012, 8:56 PM

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]]).

DanielFriesen added a comment.Via ConduitAug 13 2012, 7:59 AM

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 19204 that triggers mw-content-ready on body on DOM ready and on #wikiPreview when live-preview is used.

DanielFriesen added a comment.Via ConduitAug 13 2012, 9:33 AM

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.

Krinkle added a comment.Via ConduitAug 16 2012, 12:31 PM

(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.

DanielFriesen added a comment.Via ConduitAug 16 2012, 1:30 PM

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.

MarkAHershberger added a comment.Via ConduitSep 28 2012, 9:27 PM

Unless someone wants to do this in the next week or so, pushing.

Krinkle added a comment.Via ConduitMar 31 2013, 12:48 AM

Change-Id: Ic73a3efe53d6fb731e7f1e531d5f51530cd7e4fe

Krinkle added a comment.Via ConduitMay 22 2013, 11:11 PM

(In reply to comment #11)

Change-Id: Ic73a3efe53d6fb731e7f1e531d5f51530cd7e4fe

Merged.

Rillke added a comment.Via ConduitJul 9 2013, 11:49 AM

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.

Rillke added a comment.Via ConduitJul 9 2013, 11:57 AM

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.

Krinkle added a comment.Via ConduitJul 9 2013, 9:12 PM

(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.

gerritbot added a comment.Via ConduitJul 18 2013, 1:39 AM

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

gerritbot added a comment.Via ConduitJul 18 2013, 1:39 AM

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

gerritbot added a comment.Via ConduitJul 24 2013, 12:22 PM

Change 74312 merged by jenkins-bot:
Move firing of "wikipage.content" mw.hook out of mediawiki.util

https://gerrit.wikimedia.org/r/74312

gerritbot added a comment.Via ConduitJul 26 2013, 12:18 AM

Change 74313 merged by jenkins-bot:
mediawiki.page.ready: Use wikipage.content instead of domready

https://gerrit.wikimedia.org/r/74313

Mattflaschen added a comment.Via ConduitAug 4 2014, 9:07 PM

This doesn't depend on bug 69108. I think you meant that depends on this.

Ricordisamoa added a subscriber: Ricordisamoa.Via WebFeb 26 2015, 8:55 AM

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.