MediaWiki talk:Gadget-contribsrange.js

Archives

This page has archives. Sections older than 180 days may be automatically archived by when more than 3 sections are present.

Still necessary

Is this entire gadget still necessary now that CIDR ranges are supported natively? I note that wildcards are permitted by this gadget. It might be a good idea to remove the functionality in this gadget which is presently unnecessary. --Izno (talk) 19:52, 22 November 2017 (UTC)[reply]

@Izno: This gadget groups edits by IP, while the native implementation doesn’t. For me the grouped version is more usable. This is, of course, subjective, but I think it’s enough to justify keeping this feature in the gadget. —Tacsipacsi (talk) 12:15, 12 April 2020 (UTC)[reply]

Interface-protected edit request on 25 December 2019

This edit request has been answered. Set the |answered= parameter to no to reactivate your request.

In the comment on line 21, please replace "These parameter soverride" with "These parameters override" (i.e. move the "s" to the other side of the space). Thanks, --DannyS712 (talk) 01:36, 25 December 2019 (UTC)[reply]

Done Galobtter (pingó mió) 08:30, 25 December 2019 (UTC)[reply]

Spinner

It is requested that an edit be made to the interface page at MediaWiki:Gadget-contribsrange.js. (edit · history · last · links)

This template must be followed by a complete and specific description of the request, that is, specify what text should be removed and a verbatim copy of the text that should replace it. "Please change X" is not acceptable and will be rejected; the request must be of the form "please change X to Y".

The edit may be made by any interface administrator. Remember to change the |answered=no parameter to "yes" when the request has been accepted, rejected or on hold awaiting user input. This is so that inactive or completed requests don't needlessly fill up the edit requests category. You may also wish to use the {{EIp}} template in the response. When the request has been completed or denied, please add the |answered=yes parameter to deactivate the template.

Jon (WMF) changed the code so that it only tries to use the jQuery spinner if that’s already loaded. While this fix avoids null dereference, I don’t think it’s a good solution: if we want to show a spinner, we want to show it always, not just when it happens to have been loaded. Therefore I suggest changing

  if (mw.loader.getState('jquery.spinner') === 'ready' ) {
    $(spin).injectSpinner('prefixcontribs-spin');
  }

to

  mw.loader.using('jquery.spinner').then(function () {
    $(spin).injectSpinner('prefixcontribs-spin');
  });

which makes sure that the spinner always appears sooner or later. Thanks in advance, —Tacsipacsi (talk) 11:04, 13 September 2020 (UTC)[reply]

If

mw.loader.getState('jquery.spinner') === 'ready'

is not truthy than the next line will always throw a JS error (and was according to our production logs - a lot of errors! :)). Adding a dependency to

mw.loader.using('jquery.spinner')

is definitely the right solution, however you probably want to load it earlier in the script. If you are trying to display a spinner, it doesn't make sense to asynchronously show it - as that seems to defeat the purpose to me :) The spinner may display after the action the spinner is supposed to be displayed for as shown! Wrapping the whole code is one option, but ideally you'd wrap whatever begins the script e.g. the click of the UI button. Also consider not using jquery spinner for something so simple.

$('<img>').attr('src', urlOfCommonsSpinner ).appendTo(spin)

should work just as well without the overhead.Jon (WMF) (talk) 18:19, 13 September 2020 (UTC)[reply]

@Jon (WMF): jQuery.spinner is all about showing spinners, isn’t it? What could it do more complicated than showing a spinner? Yes, preloading the module would be even better, but I don’t know this script well enough to propose a good place for this preload. By the way, the image solution is asynchronous as well—the image needs to be loaded to show up. My favorite example is my local public transport company BKK’s timetable page: if you click on a destination, it shows a spinner GIF until the list of stops is loaded, but loading the spinner is a significant fraction of this time, so I often see the “toltes-loading” alt text (“töltés” meaning “loading” in Hungarian) for longer time than the actual spinner. —Tacsipacsi (talk) 23:01, 13 September 2020 (UTC)[reply]

Tacsipacsi, Jon (WMF) - Between the two of you, what is the right solution here? ~Oshwah~^{(talk) (contribs)} 05:53, 15 September 2020 (UTC)[reply]

@Oshwah and Jon (WMF): There’s no one Right Solution™ here, as we need to carefully balance between responsiveness and avoiding unnecessary network load: the earlier the ResourceLoader module is loaded, the higher the chance of loading it unnecessarily; the later it’s loaded, the higher the chance of the spinner not appearing immediately (or even not appearing at all, because the actual data loads faster than the spinner module).

I just looked into the code more thoroughly, and now I say more confidently that the least bad solution is loading the module right where it’s needed, that is, the solution I originally proposed (or at the beginning of that function, but that eight lines shouldn’t make a big difference). This function is called at from two places in prefixContribsInit, at the beginning of an if statement and at the beginning of its else if statement. The loading cannot be moved further up within these conditional blocks, and moving them outside of the conditional would mean the module is loaded unnecessarily in the vast majority of cases: when neither condition is true, that is, someone tries to load the contributions for a user name that is neither a CIDR range (like 127.0.0.0/24) nor a user name/IP fragment ending in an asterisk (Tacsip* or 127.0.*), for example when one clicks on the contribs link in your signature, page history, RC etc. (In this case, the script remains silent, doesn’t display anything.) —Tacsipacsi (talk) 18:56, 15 September 2020 (UTC)[reply]