Hey @Ademola, Great effort! The UI of the dialog box looks pretty good. I've tested the changes on my end and these are the things I've observed:

• The dialog box correctly appears and displays the appropriate message when we try to edit a protected file using cat-a-lot.
• You have provided 2 buttons on the dialog box, namely - "ignore and continue" and "stop all editing"
  • When I press the "ignore and continue" button, all the other files except the protected one get edited and appropriate message is displayed for the protected file. This is the correct and expected outcome.
  • When I press the "stop all editing" button, I expected all the editing to stop (even for the correct files), However what happens is that editing only stops for the protected files but it continues for all other files.
    • The cause for this issue can be understood from the screenshot i've attached. You can see that even when the user has not pressed any of the buttons, all the other files in the background already appear to have been edited. What should've happened instead is that all the other edits should be paused until user presses either of the buttons.
    • If this was the intended functionality then i think the text on the button needs to be changed to depict that appropriately.

{F58929460}

@adiba_anjum, unfortunately the attached screenshot is not visible to anyone but you. Please see https://www.mediawiki.org/wiki/Phabricator/Help#File_visibility for how to fix this.

Oops! My bad. I'll upload it again

Hi, @adiba_anjum
Thanks for testing the code and giving feedback! I appreciate the time taken to test the implementation.

Regarding the "Stop all editing" Behavior:
Currently, the implementation stops only the protected file edits while allowing the rest to proceed. This procedure ensures that the entire batch edit process is not blocked by a single protected file.
I can adjust the logic if the expected behavior is to pause all modifications until the user explicitly decides, which I don't believe it is. For additional information, see T386779, I can update the logic accordingly. Alternatively, if the current functionality is acceptable, I can modify the button text to show that only protected files will be excluded.
Thanks again for the valuable insights!

My only concern is that, at the moment, both buttons seem to lead to the same outcome—protected files remain unedited, while all other files are edited.

I just wanted to share my perspective as a user, as I wasn't aware of the intended functionality. Based on the button labels, I initially interpreted them as follows:

• Ignore and continue: The file causing the error will be ignored, and the rest of the edits will proceed.
• Stop all editing: All edits will be halted.

If the current functionality is correct as intended, perhaps a small adjustment to the button text—such as changing "Stop all editing" to "Stop editing protected files"—could make it clearer?

Just sharing my thoughts! Of course, I’ll leave the decision to you. :)

I think that current issue is maybe because Gadget-libAPI.js will runing the editing in the background when the dialog is open and when user clicks the "stop all editing" it has already edited all articles. Button kind of works as if there would be huge number of edits then editing would be going on when user clicks it and edits would stop.

In any case i need to confirm this.

@Ademola When Gadget-libAPI.js's doRequest() is executed, it will first call always() (in line 546). Only after this will the error handler launching the error dialog be executed (doErrCb() in line 606) and then errCb() (in line 554). As always() has launched the next edit from apiRequestQueue, there is a new editing thread going on in the background.

As there are multiple threads editing simultaneously, it would not be enough to pause this one, but there would need to be some check in always() if the editing is paused and also an ability to continue editing after the notification is acknowledged.

My solution to this

My solution for this would be to change the error dialog in this ticket so that the notification would only tell that the edit has failed because the page is protected. Then the user can acknowledge it, but there would be no option to cancel editing.

I would also create a separate ticket for modifying Gadget-libAPI.js so that it would support pausing the edits, and a second subticket for modifying Cat-a-lot so that it will pause the editing in case of errors after support has been added to Gadget-libAPI.

Appreciate the feedback. However, I followed the task exactly as intended. The functionality is working as designed, so if there’s any confusion, a reread of the expected behavior should clarify it. Thanks for sharing your thoughts!

In T390131#10683373, @adiba_anjum wrote:

My only concern is that, at the moment, both buttons seem to lead to the same outcome—protected files remain unedited, while all other files are edited.

I just wanted to share my perspective as a user, as I wasn't aware of the intended functionality. Based on the button labels, I initially interpreted them as follows:

• Ignore and continue: The file causing the error will be ignored, and the rest of the edits will proceed.
• Stop all editing: All edits will be halted.

If the current functionality is correct as intended, perhaps a small adjustment to the button text—such as changing "Stop all editing" to "Stop editing protected files"—could make it clearer?

Just sharing my thoughts! Of course, I’ll leave the decision to you. :)

Thanks for the breakdown.
I will look into adjusting Gadget-libAPI.js and Cat-a-lot to properly handle pausing edits should be addressed separately,as you've suggested. Let me know if there are specific details i need too pay attention to.

In T390131#10684104, @Zache wrote:

@Ademola When Gadget-libAPI.js's doRequest() is executed, it will first call always() (in line 546). Only after this will the error handler launching the error dialog be executed (doErrCb() in line 606) and then errCb() (in line 554). As always() has launched the next edit from apiRequestQueue, there is a new editing thread going on in the background.

As there are multiple threads editing simultaneously, it would not be enough to pause this one, but there would need to be some check in always() if the editing is paused and also an ability to continue editing after the notification is acknowledged.

My solution to this

My solution for this would be to change the error dialog in this ticket so that the notification would only tell that the edit has failed because the page is protected. Then the user can acknowledge it, but there would be no option to cancel editing.

I would also create a separate ticket for modifying Gadget-libAPI.js so that it would support pausing the edits, and a second subticket for modifying Cat-a-lot so that it will pause the editing in case of errors after support has been added to Gadget-libAPI.

In T390131#10684706, @Ademola wrote:

However, I followed the task exactly as intended.

There may have been errors in the assumptions in the task description by me as well. In any case, it is appreciated if you spot errors in task descriptions or have suggestions how implementation could be improved. So from my perspective, there is no problem if you say that the code should work differently, because the proposed way is not good from a usability standpoint.

In any case, I think that Adiba had a valid point on how the UI works. My reason to split the task into separate tickets is that I think it would be useful to get the error messages to production, and it would be more important than fixing the Gadget-libAPI.js inability to stop edits.

However, I may be wrong in this, and it is trivial to fix libAPI. Here is the ticket for fixing it, T390242.

Hello @Zache
I fixed the error dialog as suggested earlier.
The user can now acknowlegde the error by clicking on the ignore and continue button
I removed the stop editing button and styles.
Here:
Cat-a-lot.js - view diff

I would like to proceed by fixing the issue with Gadget-libAPI.js in T390242.
Thanks @adiba_anjum for the feedback.

@Ademola the new version could be updated to edit request too with a comment that adding the ability to pause gadget-libAPI.js edits is being worked on in separate edits and aborting will be added to the dialog when it is ready. (+ links to relevant Phabricator tickets).

Also, based on the code, it looks like the current behavior in official version is that the UI will get stuck, but editing continues in the background and adding an error message doesn't change that. However, I haven't actually confirmed this by testing it and it is only based on reading the code.

Thanks @Zache
I will update the edit request as suggested.
Looking forward to your feedback after confirming the behaviour from UI perspective.

Hello @adiba_anjum
I have updated the description for proper testing of the bug
I have made neccesary changes to fix the stop editing button
Looking forward to your feedback

Hey! Everything works well now—great job!

At first, I was running into issues because I had loaded the script into the common.js of Wikimedia Commons, but the test category mentioned in the description is on Beta Commons, so nothing was working initially. Once I realized my mistake and loaded the script correctly, everything functioned as expected.

I think it would be helpful to mention in the description that the script needs to be loaded in Beta Commons for testing, so others also don't repeat my mistake.

Thanks for the feedback so far
It was indeed helpful in fixing this bug
About the testing in BETA commons i would fix that in the description.

In T390131#10714813, @adiba_anjum wrote:

Hey! Everything works well now—great job!

At first, I was running into issues because I had loaded the script into the common.js of Wikimedia Commons, but the test category mentioned in the description is on Beta Commons, so nothing was working initially. Once I realized my mistake and loaded the script correctly, everything functioned as expected.

I think it would be helpful to mention in the description that the script needs to be loaded in Beta Commons for testing, so others also don't repeat my mistake.