Page MenuHomePhabricator

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

Details

Reference
bz30713

Event Timeline

bzimport raised the priority of this task from to High.Nov 21 2014, 11:54 PM
bzimport set Reference to bz30713.

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

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.

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;
				}
			};
		}() ),

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

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.

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.

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

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.

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

Change-Id: Ic73a3efe53d6fb731e7f1e531d5f51530cd7e4fe

(In reply to comment #11)

Change-Id: Ic73a3efe53d6fb731e7f1e531d5f51530cd7e4fe

Merged.

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.

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.

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

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

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

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

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

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

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

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