Last modified: 2009-08-15 10:07:39 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 T22125, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 20125 - Change of way to initialize editToolbarConfiguration structure
Change of way to initialize editToolbarConfiguration structure
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
UsabilityInitiative (Other open bugs)
unspecified
All All
: Normal normal (vote)
: ---
Assigned To: Trevor Parscal
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-08 03:46 UTC by Liangent
Modified: 2009-08-15 10:07 UTC (History)
2 users (show)

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


Attachments

Description Liangent 2009-08-08 03:46:24 UTC
Currently, you are using var editToolbarConfiguration = { ... to initialize it, and do it after user&site js are loaded. I hope that you can change to use jQuery.extend and load it before loading of user&site js. This will enable us to customize it easier.
Comment 1 Roan Kattouw 2009-08-08 16:35:35 UTC
It *is* easy to customize it:

addOnloadHook(function() {
        if ( typeof editToolbarConfiguration != 'undefined' ) {
                // Do stuff
        }
});

Resolving as WORKSFORME, please REOPEN if there's a compelling reason why EditToolbar.js has to be before user/site JS.
Comment 2 Liangent 2009-08-09 00:47:20 UTC
I think addOnloadHook is just something like a hack. It's available only because toolbar's initialization hooks $(document).ready.

What's the problem if the order is changed?
Comment 3 Roan Kattouw 2009-08-09 09:56:42 UTC
(In reply to comment #2)
> I think addOnloadHook is just something like a hack. It's available only
> because toolbar's initialization hooks $(document).ready.
> 
> What's the problem if the order is changed?
> 

The core code doesn't facilitate adding extension JS before user/site JS. I've fixed the issue in a different way in r54646: user/site JS can now customize the toolbar with:

$(document).bind('toolbarConfig', function() {
        // Do stuff with editToolbarConfiguration
});

(wrapped in a check for $ being defined, of course).
Comment 4 Liangent 2009-08-10 10:33:56 UTC
Will it be better if editToolbarConfiguration object is passed into the event handler? (but I don't know where it can be put)
Comment 5 Liangent 2009-08-10 10:34:32 UTC
This can avoid hard coding of variable name editToolbarConfiguration.
Comment 6 Roan Kattouw 2009-08-10 10:35:05 UTC
(In reply to comment #4)
> Will it be better if editToolbarConfiguration object is passed into the event
> handler? (but I don't know where it can be put)
> 

You'll want to change in, and you can't pass stuff by reference in JS AFAIK.
Comment 7 Liangent 2009-08-10 10:39:38 UTC
javascript:function c(x){x.a=1;}var o={};c(o);alert(o.a);

This says 1.
Comment 8 Roan Kattouw 2009-08-10 11:20:46 UTC
(In reply to comment #7)
> javascript:function c(x){x.a=1;}var o={};c(o);alert(o.a);
> 
> This says 1.
> 

Holy crap, that actually works :O Added parameter in r54711. Usage:

$(document).bind('toolbarConfig', function(event, config) {
        // config is the editToolbarConfiguration object
        // Note that it's the 2nd parameter
});
Comment 9 Roan Kattouw 2009-08-11 14:28:26 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > javascript:function c(x){x.a=1;}var o={};c(o);alert(o.a);
> > 
> > This says 1.
> > 
> 
> Holy crap, that actually works :O Added parameter in r54711. Usage:
> 
> $(document).bind('toolbarConfig', function(event, config) {
>         // config is the editToolbarConfiguration object
>         // Note that it's the 2nd parameter
> });
> 

Please note that a different API for adding, modifying and removing toolbar controls is underway and the above will soon be obsolete. Apologies for any inconvenience caused.
Comment 10 Liangent 2009-08-12 04:09:14 UTC
when will a stable api be available?
Comment 11 Roan Kattouw 2009-08-12 08:04:05 UTC
(In reply to comment #10)
> when will a stable api be available?
> 

When it's done :D

Rest assured that as long as a stable API is not available, the original hook will not go live on Wikipedia (in fact, we intend to remove it when we have the new API, so that hook will not go live on WP ever).
Comment 12 Roan Kattouw 2009-08-14 08:01:26 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > when will a stable api be available?
> > 
> 
> When it's done :D
> 
> Rest assured that as long as a stable API is not available, the original hook
> will not go live on Wikipedia (in fact, we intend to remove it when we have the
> new API, so that hook will not go live on WP ever).
> 

And it's there. Quote from bug 20134 comment #6 (where all this stuff was mis-posted, my apologies):

This is a good example of the API in use. (it's our tests to make sure it's
working)

http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/UsabilityInitiative/js/tests/wikiEditor.toolbar.js?view=markup
Comment 13 Liangent 2009-08-15 04:47:56 UTC
Why not follow some patterns like method invocation in jQuery UI?

$('#wpTextbox1').wikiEditor('add', { /* options */ });

and some more...

$('#wpTextbox1').wikiEditor('get', { index: 0 }).bind('x-click', function(event) { // avoid browser's click event
    return { /* what to do */ };
});

$('#wpTextbox1').bind('wikiEditorClick', function(event, button) {
    return { /* what to do */ };
});
Comment 14 Roan Kattouw 2009-08-15 10:07:39 UTC
(In reply to comment #13)
> Why not follow some patterns like method invocation in jQuery UI?
> 
> $('#wpTextbox1').wikiEditor('add', { /* options */ });
> 
> and some more...
> 
> $('#wpTextbox1').wikiEditor('get', { index: 0 }).bind('x-click',
> function(event) { // avoid browser's click event
>     return { /* what to do */ };
> });
> 
> $('#wpTextbox1').bind('wikiEditorClick', function(event, button) {
>     return { /* what to do */ };
> });
> 

I don't know why it wasn't done here, but I am going to use there patterns in my rewrite of mwsuggest.js to a more generic suggestions jQuery plugin.

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


Navigation
Links