Page MenuHomePhabricator

Add pause / continue to Gadget-libAPI.js
Closed, ResolvedPublic

Description

Currently, there is no possibility to temporarily stop editing in MediaWiki:Gadget-libAPI.js, and even if an interactive user input dialog is open, the editing continues in the background. As a libAPI user, I would like to get a ability to pause editing through public interface.

Implementation
Create a member functions startEditing() / stopEditing() which would start and stop executing edits from apiRequestQueue. Functions should be expoised to external use through mw.libs.commons.api interface similarly than abortPendingRequests()

Example:
mw.libs.commons.api.abortPendingRequests()

  • Mediawiki:Gadget-Cat-a-lot.js line 1460)
  • Mediawiki:Gadget-libAPI.js 881 )

Other info

  • In MediaWiki:Gadget-libAPI.js the main editing loop is in always() ( line 313 ) which will pull edits from apiRequestQueue and execute the edits.
  • In addition there maybe ongoing edits on recallme() calls which least in theory could be also stopped, but in this case we let them pass throug and do not try to stop them.

Event Timeline

Zache updated the task description. (Show Details)
Zache updated the task description. (Show Details)

Got it. I’ve started reviewing the implementation details and checking how best to integrate startEditing() and stopEditing() into Gadget-libAPI.js. I’ll update as I make progress.

Hi, I think that you may need categories with a lot of images to be able to test this, as if you edit only a small number of photos, it will edit too fast for you to see what is happening or to react. You can remove photos from these categories if you need larger batches (and if images run out in these categories, then you can add photos uploaded from Finna back to these categories).

Zache renamed this task from Add pause / continue Gadget-libAPI.js to Add pause / continue to Gadget-libAPI.js .Mar 28 2025, 3:44 AM
Zache updated the task description. (Show Details)

Noted.
Thank you for the suggestion.
I'll ensure that sufficient images are available for testing to see what happens efficiently.

Hello @Zache
I have implemented a pause/resume editing feature for MediaWiki:Gadget-libAPI.js which now prevent edits from being processed when user interaction encounters an error (i.eprotected page ). Unlike before, edits would continue processing in the background even if an interactive user input dialog was open.

Changes:
The following modifications were made:

  • isEditingEnabled flag - This boolean flag determines whether editing operations are allowed.
  • stopEditing() - Sets isEditingEnabled = false, disabling further edits.
  • startEditing() - Sets isEditingEnabled = true, re-enabling editing.
  • processQueue() - Ensures that edits are only processed if isEditingEnabled is true.
  • abortPendingRequests() - Clears apiRequestQueue, discarding all pending edit requests immediately.

To see changes
Gadget-libAPI.js- View diff

I also modified the cat-a-lot file to test changes and used alert() and confirm() dialogs instead of jquery UI:
Cat-a-lot - View diff

Since we are still not sure which one to use between the jquery UI and the alert() and confirm() dialogs, i will be looking forward to your feedback on the changes i have made.

Thank you, it tested it. It seems that if the stopping is called in error handlers after always() then there will be multiple edits before the editing is stopped. So stopping the editing in callbacks is too slow.

Another what i noticed was that if the isPaused and isEditingEnabled are prefixed with this they are in different scope than the function which is calling mw.libs.commons.api.startEditing() in callback and changing value in one scope doesn't affect to another. Without the this prefix they are in same scope.

My solution was to add config variable pauseOnError with default value false. However, if value is true then code will pause it before always() is called and error needs to acknowledge using mw.libs.commons.api.continueEditing(); to continue. I used Jquery dialog to test that it really stops and continues to run editing and this seems to work.

Here is my changes

Thank you for the feedback. I sincerely appreciate you taking the time to look over my edits. I addressed the main concerns about timing, scope, and state management by following the procedure you described. I made sure pausing occurred before always() was called, and added the pauseOnError setting to address these problems. In order to confirm that the system operated as anticipated, I also used a jQuery dialog to test the pause-and-continue capability.

I'm still not totally sure, that everything is operating flawlessly. In particular, selecting Ignore and continue causes a network request to be sent to https://intake-logging.wikimedia.org/v1/events?hasty=true when an issue occurs and the dialog box opens. I'm not sure whether this request is typical behavior or whether it's a sign of an unforeseen consequence.

I would be very grateful for your advice on the network request. I want to know specifically whether this request should happen when I click Ignore and Continue in the dialog box.

Changes
Gadget-libAPI.js- View diff
Cat-a-lot - View diff

Thank you again for your valuable feedback, and I will continue refining the implementation based on further testing and your suggestions.

I think that logging.wikimedia.org is something which is not related to your code.

However currently in your code the variable pauseOnError is initally false for backwards compatibility and it needs to be set true somewhere. In my code I am setting it true at same function where it defines how many parallel queries it is doing. (see user:Zache/cat-a-lot.js line: 1156 ) It would also require the continueEditing() to be defined somewhere so that it would work ( line 787)

Also, I think that it should work well enough to restore the "Stop all editing" button if you like to do it.

Alright
I appreciate the explanation. Given that logging.wikimedia.org is unconnected, that makes logical.

For backward compatibility, pauseOnError appears to be initially set to false. Using your method in user:Zache/cat-a-lot.js at line 1156, I'll try to set it to true at the right location, perhaps where the number of simultaneous queries is specified.
It also makes sense to restore the "Stop all editing" button. But let note that stop all editing button will stop all the editing process.

Thank you for the feedback! If you have any other recommendations, please let me know.

Hello @Zache

Thanks to your insight i have specified pauseOnError: true in the setup to enhance the pause running processes behavior. This update enables the system to halt further processes whenever an error occurs, giving users the opportunity to review the problem before making a decision. This guarantees a more controlled workflow and stops unwanted automated operations.
There also has to be a mechanism to resume operations once the user chose to proceed, since pauseOnError halts processing when an error occurs. Based on your feedback continueEditing() to fix this. If an error caused operations to be interrupted, this method determines it and then dequeues the subsequent request to restart execution.

I also put the "Stop all editing" button back, giving users the ability to end all active actions at any time if necessary. When this button is pressed, CAL.doAbort() is called, all pending tasks are cancelled, the progress dialog is deleted and UI components are reset.

Also, i noticed that the error dialog appears twice in cases where two protected files follows each bacause of:

maxSimultaneousReq: 2,

if the protected page follow each other, we will have two dialog, i changed it to maxSimultaneousReq: 1, and it was fix but i would want a feedback on if i should go on with this approach.

I created a clean diff for proper review
Cat-a-lot - View diff
Gadget-libAPI.js- View diff

Here is a screenrecord of the fix:

ScreenRecording2025-04-02at14.20.20-ezgif.com-video-to-gif-converter (1).gif (800×416 px, 3 MB)

Changing maxSimultaneousReq: 2, to 1 after the error has been happened doesn't effect to two error dialogs because both requests have already been started before the first error has been happened.

If the initial value of maxSimultaneousReq had been 1, then there would have been only 1 dialog as there is no parallel edits. However, we want to make multiple parallel edit requests to achieve a higher editing speed. So multiple dialogs are feature which comes from parallel editing, not a bug.

Changing maxSimultaneousReq: 2, to 1 after the error has been happened doesn't effect to two error dialogs because both requests have already been started before the first error has been happened.

If the initial value of maxSimultaneousReq had been 1, then there would have been only 1 dialog as there is no parallel edits. However, we want to make multiple parallel edit requests to achieve a higher editing speed. So multiple dialogs are feature which comes from parallel editing, not a bug.

Alright.
I will take note of that

Hello @Zache
Concerning the code i'm curious and want to know if they have being test and ready to merge
Thank you.

Yeah, I think functionality is OK.

Only thing what i would still clean is that in Gadget-libAPI.js diff there is unneeded var always = function () to function always() { change and then in always() function has whitespace changes which are visible in diff.

Removing unnecessary changes from the diff is so that whoever is merging the changes into the production version doesn't have to try to understand by reading the code which changes produce functional changes and which don't.

You could also update ticket T390131 so that all links, etc. are pointing to the correct places, allowing it to be linked as a description on how to test the change.

Alright Noted
I will do that and get back to you
Also can this ticket be added to Outreachy 30: Cat-a-lot javascript gadget development
I would also update T390131
Thanks for the feedback.

Yeah, I think functionality is OK.

Only thing what i would still clean is that in Gadget-libAPI.js diff there is unneeded var always = function () to function always() { change and then in always() function has whitespace changes which are visible in diff.

Removing unnecessary changes from the diff is so that whoever is merging the changes into the production version doesn't have to try to understand by reading the code which changes produce functional changes and which don't.

Hello @Zache
I've made neccessary changes and this is ready for to merge
I changed function always() {} to var always = function ()
I removed unneccessary whitespace too
Have also updated T390131 description for proper testing.
Thanks for your guidance!

I created a clean diff for proper review
Cat-a-lot - View diff
Gadget-libAPI.js- View diff

Just some comments more. I noticed couple things when I tested the error dialog with mobile.

Dialog close bug:
I just noticed that it is possible to close the error dialog from top-right corner X. If user does so then Cat-a-lot it will be stuck as no handlers are called. The closing button in right-top corner could be removed or it could just continue similarly than in ignore and continue button.

  • Mobile screen**

Visually error dialog worked OK and it was perfectly usable but there could be little padding in left side as now text starts directly from border of the screen.

Howto test different skins

You can simulate Wikimedia Commons mobile view on desktop by changing the skin as "MinervaNeye" ( Preferences -> Appearance ) and narrow the browser window. There is also url parameter useskin=minerva ( https://commons.wikimedia.org/wiki/Special:Search?useskin=minerva ) which switches the skin temporarily.

  • Whitespaces**

There could be also as some whitespace tuning just for polishing the code

Whitespaces could be identical with the original so unneeded changes will go away from the diff in these.

Indentation should be the same as in the rest of the code so that the changes can be copied to the official version without having to fix the indentation manually

Alright Noted i will fix that

Just some comments more. I noticed couple things when I tested the error dialog with mobile.

Dialog close bug:
I just noticed that it is possible to close the error dialog from top-right corner X. If user does so then Cat-a-lot it will be stuck as no handlers are called. The closing button in right-top corner could be removed or it could just continue similarly than in ignore and continue button.

  • Mobile screen**

Visually error dialog worked OK and it was perfectly usable but there could be little padding in left side as now text starts directly from border of the screen.

Howto test different skins

You can simulate Wikimedia Commons mobile view on desktop by changing the skin as "MinervaNeye" ( Preferences -> Appearance ) and narrow the browser window. There is also url parameter useskin=minerva ( https://commons.wikimedia.org/wiki/Special:Search?useskin=minerva ) which switches the skin temporarily.

  • Whitespaces**

There could be also as some whitespace tuning just for polishing the code

Whitespaces could be identical with the original so unneeded changes will go away from the diff in these.

Indentation should be the same as in the rest of the code so that the changes can be copied to the official version without having to fix the indentation manually

Hello @Zache

Changes made based on your feedback:

  1. Fixed the dialog close issue: Remove the close the error dialog from top-right corner X
  2. Improved mobile styling: Added left padding to the error dialog on small screens. Previously, text was flush against the screen edge. But i'm not sure if i could use mobile-responsive media query to fix that, However it worked please let me know If this complies with MediaWiki's CSS guidelines.
@media screen and (max-width: 480px) {
    .cat-a-lot-error-dialog {
        width: 100% !important;
        padding-left: -20px;
    }
}

Screenshot of the mobile view:
Confirmed visually on mobile using useskin=minerva.

Screenshot 2025-04-08 at 02.01.41.png (1,264×1,357 px, 199 KB)

  1. Whitespace + indentation cleanup: Removed unnecessary whitespace and aligned indentation with the rest of the codebase to keep the diff clean.

Here is the diff for review
Gadget-Cat-a-lot.js - View diff
Gadget-libAPI.js- View diff
Cat-a-lot.css - View diff

merged in to main code as part of the T395770