r73150 MediaWiki - Code Review archive

-$wgWikiEditorModules = array(
+$wgWikiEditorFeatures = array(

Why are you breaking backwards compatibility like that?

+				foreach ( $feature['preferences'] as $key => $options )
+				$defaultPreferences[$key] = $options;

This is indented wrong.

+	// Some toolbar tools depend on knowing if dialogs have been enabled or not. By adding a class to the textarea, we
+	// provide a way that the the 'filters' property can be used.
+	if ( mediaWiki.user.options.get( 'usebetatoolbar-cgd' ) ) {
+		if (
+			typeof $.wikiEditor.modules.dialogs !== 'undefined' &&
+			$.wikiEditor.isSupported( $.wikiEditor.modules.dialogs )
+		) {
+			$( '#wpTextbox1' ).addClass( 'toolbar-dialogs' );
+		}
+	}

After having moved this code to the dialogs module, I can't help but think that this only works by accident, i.e. because the dialogs module is loaded before the toolbar module. If they're executed in reverse order, the toolbar module will make decisions based on the absence of the toolbar-dialogs class only to have the dialogs module add it after that. Could you test whether this does indeed depend on execution order?

I'm not a fan of braking backwards compatibility, but since we used the word modules in ResourceLoader, it starts getting really confusing when we also use it within WikiEditor with a different meaning, especially when both concepts are intersecting. So, this was a carefully made decision which I hope will not cause too many problems. It's also a change that's well documented on Extension:WikiEditor.

The indentation and bracing issue is fixed in r74086.

OK, so I'm going to be re-factoring WikiEditor next week, and this will be solved. I'm also going to pull the iframe stuff into a module so there's not so much bloat.

Good point. Documenting it in the README is probably a good idea too. I guess it's fair to expect people to rename the config var i they have to change the ext path to include from anyway.

r74114 addresses the timing issues by having the dialogs module replace the non-dialog buttons upon loading. I also brought back on-demand loading of jquery.ui stuff.