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]

Spinner

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

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]

@Jon (WMF): are you happy with the approach above and if so, can one of you post a link to a sandbox with the code required? — Martin (MSGJ · talk) 08:12, 22 September 2020 (UTC)[reply]

@MSGJ: I hope Jon will be happy with my version (diff). I originally implemented the suggested solution, but while testing it for the race condition, I realized that when it occurs (i.e. the API query is faster than loading the spinner ResourceLoader module), the spinner spins forever without a possibility for the average user to remove it (other than reloading the whole page), which is pretty annoying. :/ So I went for a more race condition-safe solution, which stores the load progress in a global (gadget-scope) variable. I also added a lot of function documentation—I hope they will help the next person trying to understand how this script works, and also IDEs like Visual Studio Code to provide better code completion. (Please delete my subpage once the code goes live, I’d like to keep my user space clean.) —Tacsipacsi (talk) 21:44, 24 September 2020 (UTC)[reply]

Deployed — Martin (MSGJ · talk) 21:50, 24 September 2020 (UTC)[reply]

Bug regarding this gadget

When I open up Special:Contributions/Jules*, the page immediately starts searching contribs as if I was looking up an IP address with an asterisk at the end. It's pretty disruptive to look at, since there are a LOT of users whose username starts with Jules. Wondering if this could be addressed? bibliomaniac 1 5 01:14, 13 October 2021 (UTC)[reply]

The problem is that searching with "username * " is part of the advertised functionality of this tool. I guess you could check if the user exists, and stop the script from running if it does, but then you wouldn't be able to actually search for users with prefixed "Jules" in their username. You can disable the tool in Preferences > Gadgets > Advanced. Dylsss^{(talk contribs)} 19:32, 18 October 2021 (UTC)[reply]

Bug non-vector skins

zh:Special:Diff/68628552 (for Timeless skin). --AnYiLin^Talk 13:11, 12 November 2021 (UTC)[reply]