r57968 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r57967‎ | r57968 | r57969 >
Date:08:15, 21 October 2009
Author:siebrand
Status:ok (Comments)
Tags:
Comment:
(bug 20847) Remove akeytt() function, but leave dummy. Contributed by Derk-Jan Hartman
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/skins/common/ajaxwatch.js (modified) (history)
  • /trunk/phase3/skins/common/wikibits.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/ajaxwatch.js
@@ -2,7 +2,7 @@
33 // * ajax.js:
44 /*extern sajax_init_object, sajax_do_call */
55 // * wikibits.js:
6 - /*extern changeText, akeytt, hookEvent, jsMsg */
 6+ /*extern changeText, hookEvent, jsMsg */
77
88 // These should have been initialized in the generated js
99 /*extern wgAjaxWatch, wgPageName */
@@ -44,7 +44,6 @@
4545 wgAjaxWatch.setLinkID = function( newId ) {
4646 // We can only set the first one
4747 wgAjaxWatch.watchLinks[0].parentNode.setAttribute( 'id', newId );
48 - akeytt(newId); // update tooltips for Monobook
4948 };
5049
5150 wgAjaxWatch.setHref = function( string ) {
Index: trunk/phase3/skins/common/wikibits.js
@@ -350,63 +350,8 @@
351351 return str;
352352 }
353353
354 -
355 -/**
356 - * Set up accesskeys/tooltips from the deprecated ta array. If doId
357 - * is specified, only set up for that id. Note that this function is
358 - * deprecated and will not be supported indefinitely -- use
359 - * updateTooltipAccessKey() instead.
360 - *
361 - * @param mixed doId string or null
362 - */
 354+/* Dummy for deprecated function */
363355 function akeytt( doId ) {
364 - // A lot of user scripts (and some of the code below) break if
365 - // ta isn't defined, so we make sure it is. Explictly using
366 - // window.ta avoids a "ta is not defined" error.
367 - if (!window.ta) window.ta = new Array;
368 -
369 - // Make a local, possibly restricted, copy to avoid clobbering
370 - // the original.
371 - var ta;
372 - if ( doId ) {
373 - ta = [doId];
374 - } else {
375 - ta = window.ta;
376 - }
377 -
378 - // Now deal with evil deprecated ta
379 - var watchCheckboxExists = document.getElementById( 'wpWatchthis' ) ? true : false;
380 - for (var id in ta) {
381 - var n = document.getElementById(id);
382 - if (n) {
383 - var a = null;
384 - var ak = '';
385 - // Are we putting accesskey in it
386 - if (ta[id][0].length > 0) {
387 - // Is this object a object? If not assume it's the next child.
388 -
389 - if (n.nodeName.toLowerCase() == "a") {
390 - a = n;
391 - } else {
392 - a = n.childNodes[0];
393 - }
394 - // Don't add an accesskey for the watch tab if the watch
395 - // checkbox is also available.
396 - if (a && ((id != 'ca-watch' && id != 'ca-unwatch') || !watchCheckboxExists)) {
397 - a.accessKey = ta[id][0];
398 - ak = ' ['+tooltipAccessKeyPrefix+ta[id][0]+']';
399 - }
400 - } else {
401 - // We don't care what type the object is when assigning tooltip
402 - a = n;
403 - ak = '';
404 - }
405 -
406 - if (a) {
407 - a.title = ta[id][1]+ak;
408 - }
409 - }
410 - }
411356 }
412357
413358 var checkboxes;
@@ -967,7 +912,6 @@
968913 doneOnloadHook = true;
969914
970915 updateTooltipAccessKeys( null );
971 - akeytt( null );
972916 setupCheckboxShiftClick();
973917 sortables_init();
974918
Index: trunk/phase3/RELEASE-NOTES
@@ -564,16 +564,21 @@
565565 false on non-special pages
566566 * (bug 21113) "Other statistics" header on Special:Statistics is no more
567567 displayed when there isn't any entry in it
568 -* (bug 21114) Special:Contributions no longer shows diff links for new revisions
 568+* (bug 21114) Special:Contributions no longer shows diff links for new
 569+ revisions
569570 * (bug 21116) MediaWiki:Templatesused, MediaWiki:Templatesusedpreview and
570571 MediaWiki:Templatesusedsection now support plural
571 -* (bug 21079) There is no more line wrapping between label and field in Special:Log
572 -* (bug 20256) Fixed SQL errors on Special:Recentchanges and Special:Recentchangeslinked
573 - on SQLite backend
 572+* (bug 21079) There is no more line wrapping between label and field in
 573+ Special:Log
 574+* (bug 20256) Fixed SQL errors on Special:Recentchanges and
 575+ Special:Recentchangeslinked on SQLite backend
574576 * (bug 20880) Fixed updater failure on SQLite backend
575577 * (bug 21182) Fixed invalid HTML in Special:Listgrouprights
576 -* (bug 20242) Installer no longer promts for user credentials for SQLite databases
 578+* (bug 20242) Installer no longer promts for user credentials for SQLite
 579+ databases
577580 * (bug 20911) Installer failed to create a SQLite database
 581+* (bug 20847) Deprecated deprecated akeytt() removed in wikibits.js leaving
 582+ dummy
578583
579584 == API changes in 1.16 ==
580585

Comments

#Comment by Simetrical (talk | contribs)   19:26, 12 April 2010

I think we should revert this. This was originally the only way to change accesskeys and tooltips, prior to 2007 or so. It's likely that tons of wikis got used to doing it this way and never switched to the new system – it's only been three years or so. We should give this a few more years before removing it.

#Comment by Nikerabbit (talk | contribs)   14:27, 13 April 2010

What is the point of breaking it in 2012 instead of now if we are not even trying to help getting them fixed.

#Comment by Simetrical (talk | contribs)   16:30, 13 April 2010

It's like 25 lines of actual code. As I said for IE5 support, we should keep it until we have some solid reason to get rid of it, like it no longer works anyway for some reason and nobody can be bothered to fix it. There's no reason we can't keep it forever if necessary, until we have some actual reason to delete it beyond distaste for clutter. I'm all for removing clutter, but not if it breaks hundreds or thousands of user scripts that have been working for years, not if it's this few lines.

#Comment by Nikerabbit (talk | contribs)   10:21, 14 April 2010

Basically I don't disagree. However I would like to have more active approach than let it bitrot until it is broken and then remove. Can we somehow

  • clearly mark the code as such it is not and should not be maintained
  • decide a beforehand when it is going to be removed if it does not bitrot fast enough
  • help wiki administrators locate and remove anything that depends on it

?

#Comment by Simetrical (talk | contribs)   16:08, 14 April 2010

I think we should have just marked it "kept for backward compatibility, do not remove unless you're sure you aren't going to break hundreds of user scripts" and then left it indefinitely. We don't need to help people remove things depending on it, or remove it before it's really and truly bitrotted – it's not worth it.

However, now that it's been removed for a few days, I don't see any real point in re-adding it, since presumably most of the important scripts have been fixed by now anyway. I'd like to avoid this kind of thing in the future, though. Removing old cruft is fine, but not if it a) is harmless and b) is widely used by callers we don't control/can't fix directly.

Status & tagging log