Page MenuHomePhabricator

Integrate edit-in-sequence inside ProofreadPage
Open, Needs TriagePublicFeature

Description

Currently, the WikiEditor UI used by ProofreadPage requires context switches and long wait times when editing pages one after another inside a book. eis.js is a userscript/gadget developed by the Italian Wikisource to allow users to edit pages in sequence without having to wait for page reloads and saves. It does this by asynchronously calling the Wikimedia API to perform various actions such as previewing, saving and moving between pages. Being able to integrate the features offered by edit-in-sequence (eis.js) will alow a larger audience to use the features offered by edit-in-sequence and benefit from the speed improvements that provides over normal sequential editing.

General roadmap:

  • Implement feature flag (and enable it on Beta Wikisource)
  • Implement major UI elements
    • Save
    • Page status
    • Page navigation
    • Preview
    • Edit Summary
    • Save and navigate
  • Figure out Openseadragon implementation
  • Enable as BetaFeature, work on feedback/nice to have features

Documentation: https://www.mediawiki.org/wiki/Extension:Proofread_Page/Edit-in-Sequence

Details

Subject Repo Branch Lines +/-
mediawiki/extensions/ProofreadPage master +17 -5
operations/mediawiki-config master +1 -0
mediawiki/extensions/ProofreadPage master +85 -3
operations/mediawiki-config master +1 -3
mediawiki/extensions/ProofreadPage master +157 -29
mediawiki/extensions/ProofreadPage master +136 -3
mediawiki/extensions/ProofreadPage master +150 -3
mediawiki/extensions/ProofreadPage master +43 -7
mediawiki/extensions/ProofreadPage master +91 -1
mediawiki/extensions/ProofreadPage master +15 -5
mediawiki/extensions/ProofreadPage master +39 -9
mediawiki/extensions/ProofreadPage master +490 -11
mediawiki/extensions/ProofreadPage master +113 -81
mediawiki/extensions/ProofreadPage master +28 -9
mediawiki/extensions/ProofreadPage master +302 -144
mediawiki/extensions/ProofreadPage master +142 -1
mediawiki/extensions/ProofreadPage master +183 -2
mediawiki/extensions/ProofreadPage master +190 -6
mediawiki/extensions/ProofreadPage master +78 -1
mediawiki/extensions/ProofreadPage master +1 K -61
mediawiki/core master +5 -3
operations/mediawiki-config master +3 -0
mediawiki/extensions/ProofreadPage master +99 -3
mediawiki/extensions/ProofreadPage master +29 -6
Show related patches Customize query in gerrit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 868779 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] EditInSequence: Add save module

https://gerrit.wikimedia.org/r/868779

Test wiki created on Patch demo by Sohom data using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/e14cec28c9/w

Test wiki on Patch demo by Sohom data using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/e14cec28c9/w/

Change 868779 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] EditInSequence: Add save module

https://gerrit.wikimedia.org/r/868779

Change 870710 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Add tag to edits made using EditInSequence

https://gerrit.wikimedia.org/r/870710

Change 870710 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Add tag to edits made using EditInSequence

https://gerrit.wikimedia.org/r/870710

Change 873015 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Add error message when edit fails to save

https://gerrit.wikimedia.org/r/873015

Currently the basic MVP should work on Beta Wikisource, mostly.

There are two items included in the original version of EIS that still have not been included, namely:

  • Navigate to any page other than prev/next
  • Preload text layers (and investigate integrating OCR data via Wikimedia OCR)

Change 873015 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Add error message when edit fails to save

https://gerrit.wikimedia.org/r/873015

Change 884458 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Add internal API to retrieve default content for a Page: page

https://gerrit.wikimedia.org/r/884458

Change 884468 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Persist EditInSequence dialog preferences across page loads

https://gerrit.wikimedia.org/r/884468

Change 884458 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Add internal API to retrieve default content for a Page: page

https://gerrit.wikimedia.org/r/884458

Change 884468 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Persist EditInSequence dialog preferences across page loads

https://gerrit.wikimedia.org/r/884468

Change 923765 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Improve unsaved edit management for EditInSequence

https://gerrit.wikimedia.org/r/923765

Current stuff left to implement:

Change 926831 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Add ability to view diffs in EditInSequence

https://gerrit.wikimedia.org/r/926831

Change 923765 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Improve unsaved edit management for EditInSequence

https://gerrit.wikimedia.org/r/923765

Regarding the localStorage saving of EiS data: I'm working on a new feature in core, Edit Recovery, which is about saving in-progress edit form data to indexedDB. I'm wondering if it could perhaps replace the EiS functionality, and work in EiS and normal proofreading as well (as well as everywhere else on a wiki). I'm not quite sure what this would look like yet, but probably a matter of EiS calling something when it switches between pages, so that EditRecovery can work with the correct page name. I'll add some more info in T342200.

Regarding the localStorage saving of EiS data: I'm working on a new feature in core, Edit Recovery, which is about saving in-progress edit form data to indexedDB. I'm wondering if it could perhaps replace the EiS functionality, and work in EiS and normal proofreading as well (as well as everywhere else on a wiki). I'm not quite sure what this would look like yet, but probably a matter of EiS calling something when it switches between pages, so that EditRecovery can work with the correct page name. I'll add some more info in T342200.

Yep, this would be a great idea, especially given that Edit Recovery is expected to work across content models

Change 941776 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] EIS: Add fallback text integration to EditInSequence

https://gerrit.wikimedia.org/r/941776

We intend to deploy EditInSequence on all Wikisources starting from the Wikimania week to get more feedback on the tool :)

Stuff that's on the charts in terms of improvements:

Also noting that there are a few incompatibilities with existing gadget documented here which we discovered while beta-testing.

Change 947883 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[operations/mediawiki-config@master] Enable EditInSequence on all wikisources

https://gerrit.wikimedia.org/r/947883

Change 947883 merged by jenkins-bot:

[operations/mediawiki-config@master] Enable EditInSequence on all wikisources

https://gerrit.wikimedia.org/r/947883

Mentioned in SAL (#wikimedia-operations) [2023-08-15T07:20:14Z] <taavi@deploy1002> Started scap: Backport for [[gerrit:947883|Enable EditInSequence on all wikisources (T308098)]]

Mentioned in SAL (#wikimedia-operations) [2023-08-15T07:21:53Z] <taavi@deploy1002> soda and taavi: Backport for [[gerrit:947883|Enable EditInSequence on all wikisources (T308098)]] synced to the testservers mwdebug2002.codfw.wmnet, mwdebug2001.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug1002.eqiad.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Mentioned in SAL (#wikimedia-operations) [2023-08-15T07:33:43Z] <taavi@deploy1002> Finished scap: Backport for [[gerrit:947883|Enable EditInSequence on all wikisources (T308098)]] (duration: 13m 29s)

Erm. Since this shows up, unconditionally, as a big honkin' extra pseudo-tab in the prime position up top… Has this been communicated to the communities so they know what it is, how to use it, where to report problems, etc.?

Hello, warning the communities in advance would have been great.... we have editors on the Scriptorium, trying to understand what it is and how it works, and who are very much annoyed... and no admin knew about this.

On frws, I detected at least 2 incompatibilities with standard gadgets, that have annoying consequences : while the editor may think the edit has been saved, it has not, in some cases, and in some others, it prevents the gadget to normally load data.

Maybe you could deploy as beta (on all ws projects) so that each project can check compatibility with existing standard gadgets ?

In T308098#9092646, @Xover wrote:

Erm. Since this shows up, unconditionally, as a big honkin' extra pseudo-tab in the prime position up top… Has this been communicated to the communities so they know what it is, how to use it, where to report problems, etc.?

@Xover As of right now, no, I intended to post a Mass message (on all Wikisources) however haven't gotten round to it :( The current place to report bugs is here, or on meta:Talk:Wikisource EditInSequence or physically to me on Wikimania. The assumption is that, by having it only be a tab, (instead of loading directly into the editing interface etc) it would give the community time to test incompatible gadgets and such and make changes.

Hello, warning the communities in advance would have been great.... we have editors on the Scriptorium, trying to understand what it is and how it works, and who are very much annoyed... and no admin knew about this.

On frws, I detected at least 2 incompatibilities with standard gadgets, that have annoying consequences : while the editor may think the edit has been saved, it has not, in some cases, and in some others, it prevents the gadget to normally load data.

This is something that is somewhat already known. There is a patch that is supposed to be deployed in tomorrow's train that should fix this issue for some gadgets, if not, I can help with debugging and fixing them if required. (Feel free to ping me in the discussion, I'm User:Sohom_Datta onwiki (@Xover as well))

Maybe you could deploy as beta (on all ws projects) so that each project can check compatibility with existing standard gadgets ?

This is the beta (to a certain extent), if stuff doesn't work, you are not forced to use it. Having a beta feature that enables a button that enables a feature isn't really a usefull way of implementing this in my opinion

In T308098#9093260, @Soda wrote:

This is the beta (to a certain extent), if stuff doesn't work, you are not forced to use it. Having a beta feature that enables a button that enables a feature isn't really a usefull way of implementing this in my opinion

I disagree. It's not "only a tab": it's a tab, in the main part of the interface. That it is beta is still more reason it should have a "softer" deployment. The usual course is to start by making it opt-in in the "Beta features" section of Special:Preferences, and then send out a mass message to the Scriptoriums that explains what it is and asks for feedback. That lets you safely evaluate whether this is something that can be enabled everywhere or whether it needs some other process; and it gives the projects a chance to evaluate the impact this is going to have (technical, process, guidance, etc.).

In T308098#9093484, @Xover wrote:
In T308098#9093260, @Soda wrote:

This is the beta (to a certain extent), if stuff doesn't work, you are not forced to use it. Having a beta feature that enables a button that enables a feature isn't really a usefull way of implementing this in my opinion

I disagree. It's not "only a tab": it's a tab, in the main part of the interface. That it is beta is still more reason it should have a "softer" deployment. The usual course is to start by making it opt-in in the "Beta features" section of Special:Preferences, and then send out a mass message to the Scriptoriums that explains what it is and asks for feedback. That lets you safely evaluate whether this is something that can be enabled everywhere or whether it needs some other process; and it gives the projects a chance to evaluate the impact this is going to have (technical, process, guidance, etc.).

@Xover We did perform extensive beta tests on itwikisource, napwikisource and pawikisource (where we posted on the Scriptorium asking for feedback) we also advertised the deployment on en.wikisource.beta.wmflabs.org on the monthly Wikisource Community Meetings and only released it once we had fixes for most of the critical issues raised. (The gadget patch was supposed to be deployed last week, but I miscalculated and since there was no version of MediaWiki deployed last week, it is going to be deployed tmrw).

Also wrt to your point of this being "beta", it's mostly complete, barring certain issues that already have patches/are in the process of being patched. Burying it behind a additonal 4 click beta feature modal is going to delay the bug finding process even more. Having it be a additional tab that you can use (similar to the way VisualEditor works) seemed (atleast in my POV) a good middle ground to get the most amount of feedback so that we could start fixing any remaining bugs that are left :)

In T308098#9093260, @Soda wrote:

This is the beta (to a certain extent), if stuff doesn't work, you are not forced to use it. Having a beta feature that enables a button that enables a feature isn't really a usefull way of implementing this in my opinion

A tab being there, without a “beta” label, doesn’t sound like a beta phase. Imagine a non-techy user seeing it: would they think it’s a beta feature? If it’s “buried behind an additional 4-click beta feature modal”, they would. While, of course, one isn’t forced to use it, the impression is that it can safely be used, without glitches and definitely without data loss. Releasing it as a beta feature and after that sending mass messages to every Wikisource would have made it much less likely that inexperienced users see malfunctioning gadgets or other bugs.

In T308098#9093260, @Soda wrote:

This is the beta (to a certain extent), if stuff doesn't work, you are not forced to use it. Having a beta feature that enables a button that enables a feature isn't really a usefull way of implementing this in my opinion

A tab being there, without a “beta” label, doesn’t sound like a beta phase. Imagine a non-techy user seeing it: would they think it’s a beta feature? If it’s “buried behind an additional 4-click beta feature modal”, they would. While, of course, one isn’t forced to use it, the impression is that it can safely be used, without glitches and definitely without data loss. Releasing it as a beta feature and after that sending mass messages to every Wikisource would have made it much less likely that inexperienced users see malfunctioning gadgets or other bugs.

@Tacsipacsi I misspoke in that comment. Please take a look at the comment above as to what I meant wrt to beta in that specific context, the feature is mostly complete barring a few issue (shortcuts and text preloading/OCR integration that I'm already in the process of writing patches for)

That gadget issues are mostly due to a miscalculation in the timing of when the patch was going to be deployed, sorry for the issues

Change 949485 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Dont show multiple unsaved edits indicators

https://gerrit.wikimedia.org/r/949485

In T308098#9093741, @Soda wrote:

We did perform extensive beta tests on itwikisource, napwikisource and pawikisource (where we posted on the Scriptorium asking for feedback) we also advertised the deployment on en.wikisource.beta.wmflabs.org on the monthly Wikisource Community Meetings

itWS, napWS, and paWS are not representative of all Wikisourcen, and that there has been communication with three communities does nothing for the hundred or so other communities. The monthly Wikisource Community Meetings is a narrow self-selected group whose scheduling excludes some contributors, and whose non-privacy-protecting platform excludes others. Decisions made in either of these venues excludes the majority of stakeholders, and communicating with these is insufficient to reach the majority of stakeholders.

and only released it once we had fixes for most of the critical issues raised.

The issue is not the development need for testing to enable bug fixing. It is 1) from where you are getting input on the fundamental design in a phase where it can actually be adjusted at a reasonable cost, and 2) who is being informed of upcoming changes to core workflow on their projects.

Note: none of the above should be understood as opposition to the feature as such or any of the forums mentioned (I'm very happy someone is working on integrating a real eis.js, and the Community Meeting is great for what it is). This is only intended as feedback about how the feature is being deployed and communicated about.

Minor UI issue: the icons in the Page status dropdown do not appear to use the same colors as Proofread Page. The icon for the "Problematic" status is more blue than purple, and without actually checking color values the other page statuses look to me like they're different shades of red/yellow/green.

In T308098#9099917, @Xover wrote:

Minor UI issue: the icons in the Page status dropdown do not appear to use the same colors as Proofread Page. The icon for the "Problematic" status is more blue than purple, and without actually checking color values the other page statuses look to me like they're different shades of red/yellow/green.

That was a somewhat intentional descision on my end, the current colors are not accessible/contrasty (they do not pass WCAG AA in their current configuration, and don't work against a white background). These were the closest colors to our current color set (visually speaking) to the colors found in the Wikimedia style guidelines.

I would put in a proposal/patch to change the current pagelists to use this color scheme/icon set as well (for consistency), but have refrained from doing that, due to it being a controversial change that would require agreement from multiple wikis.

In T308098#9099990, @Soda wrote:

I would put in a proposal/patch to change the current pagelists to use this color scheme/icon set as well (for consistency), but have refrained from doing that, due to it being a controversial change that would require agreement from multiple wikis.

I agree that this would be a controversial change that needs community-involvement and a consensus process, but 1) it's not really any less controversial to do so in EIS's UI and 2) base PRP and EIS should use the same colors for the page statuses whatever they are.

The "Go to specific page" function is nifty, and having filters for page status there is very handy for specific situations, but I think it would be much clearer and easier to navigate it if the pages in the pagelist widget were colored with the page status color directly. On enWS we also have a Gadget that adds an additional border to each page box on the Index page to indicate whether the current user is able to, e.g., Validate that page. The obvious use case for this is having an overview of the pagelist because right now you're working on a specific set of pages: but that set could equally well be a combination like "All redlinks plus all Not Proofread" or "All Not Proofread and Problematic". Coloring the boxes directly also gives you a "grand overview".

(And as a corollary: regardless of what you implement directly in EIS/pagelist widget here, at some point someone is going to want to manipulate the page boxes along these lines for some reason. Making sure there is some sane way to do that is probably a good idea.)

Change 884458 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Add internal API to retrieve default content for a Page: page

https://gerrit.wikimedia.org/r/884458

Incidentally, I have (narrow and obscure, but real) use cases for access to the original text layer. Given we host the underlying file with the text layer on Commons, and shove it to the user through the edit box on new page creations, I don't think the "potentially untrusted" content of the text layer is any reason to mark the API endpoint to retrieve it as internal-only.

The docs don't seem to show any .ready() type hooks, any (documented) way to add .onSave() type handlers, or attach handlers to the page status change widget.

On enWS we have one Gadget that converts LST XML-like syntax to a simplified (##) syntax on page load, and back again on save and diff. It needs to know when the text layer is loaded so it can do the replace it, and it needs to do a "pre-save transform" when the user saves the page.

I also have a (currently user script soon to be Gadget) that empties the footer/body/header text fields when the user clicks "No Text" (and restores them if they click something else). It needs to be able to install similar hooks when EIS is active.

I'm a little uncertain as yet how EIS treats saving / loading so I'm not entirely sure what the API needs for the mentioned Gadgets will be in this context, but I'll want to adapt them to perform equivalent functionality in a way that makes sense in the context of EIS.

In T308098#9100103, @Xover wrote:

The "Go to specific page" function is nifty, and having filters for page status there is very handy for specific situations, but I think it would be much clearer and easier to navigate it if the pages in the pagelist widget were colored with the page status color directly. On enWS we also have a Gadget that adds an additional border to each page box on the Index page to indicate whether the current user is able to, e.g., Validate that page. The obvious use case for this is having an overview of the pagelist because right now you're working on a specific set of pages: but that set could equally well be a combination like "All redlinks plus all Not Proofread" or "All Not Proofread and Problematic". Coloring the boxes directly also gives you a "grand overview".

(And as a corollary: regardless of what you implement directly in EIS/pagelist widget here, at some point someone is going to want to manipulate the page boxes along these lines for some reason. Making sure there is some sane way to do that is probably a good idea.)

I would recommend using the PageSelection API to do this

In T308098#9100117, @Xover wrote:

Change 884458 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Add internal API to retrieve default content for a Page: page

https://gerrit.wikimedia.org/r/884458

Incidentally, I have (narrow and obscure, but real) use cases for access to the original text layer. Given we host the underlying file with the text layer on Commons, and shove it to the user through the edit box on new page creations, I don't think the "potentially untrusted" content of the text layer is any reason to mark the API endpoint to retrieve it as internal-only.

Feel free to use the API if you need to, provided you don't add the text directly to the DOM (if you do that, it's a security issue). The reason it is marked internal only is to discourage widespread indiscriminate use of the API outside of ProofreadPage, since the API does return untrusted content that might cause a cross-site scripting vulnerability if used incorrectly.

The original eis would load the raw page and extract this information from the associated DOM, so the information will never be not available :)

In T308098#9100085, @Xover wrote:
In T308098#9099990, @Soda wrote:

I would put in a proposal/patch to change the current pagelists to use this color scheme/icon set as well (for consistency), but have refrained from doing that, due to it being a controversial change that would require agreement from multiple wikis.

I agree that this would be a controversial change that needs community-involvement and a consensus process, but 1) it's not really any less controversial to do so in EIS's UI and 2) base PRP and EIS should use the same colors for the page statuses whatever they are.

I intend to propose these changes at some point, but from my thought process at that time was that I would prefer a more accessible slightly wrong widget v/s one that I personally need to squint to see properly :(

In T308098#9100136, @Xover wrote:

The docs don't seem to show any .ready() type hooks, any (documented) way to add .onSave() type handlers, or attach handlers to the page status change widget.

That is a somewhat interesting usecase that I hadn't thought about yet, we would definitely want to add events for those :) EIS does use the postEdit callback, so that might be a good place to start ?

On enWS we have one Gadget that converts LST XML-like syntax to a simplified (##) syntax on page load, and back again on save and diff. It needs to know when the text layer is loaded so it can do the replace it, and it needs to do a "pre-save transform" when the user saves the page.

I would be somewhat hesitant to add a "pre-save transform" hook here, mainly since that could lead to a bunch of race-conditions especially since EIS has no way of knowing when the script finishes without inventing a JS nightmare :(

I also have a (currently user script soon to be Gadget) that empties the footer/body/header text fields when the user clicks "No Text" (and restores them if they click something else). It needs to be able to install similar hooks when EIS is active.

That seems like something we would want to support, I will start working on a patch to transperantly expose this information

I'm a little uncertain as yet how EIS treats saving / loading so I'm not entirely sure what the API needs for the mentioned Gadgets will be in this context, but I'll want to adapt them to perform equivalent functionality in a way that makes sense in the context of EIS.

I would personally refrain from manipulating it right now, Edit Recovery is coming soon, and we could potentially transform away from our current approach to one that is a wrapper around EditRecovery.

In T308098#9100738, @Soda wrote:

I would recommend using the PageSelection API to do this

My point was that filters, while great for other things, is not the best model for the kinds of use cases I mentioned. Color coding the page boxes in the widget would obviate the need for a large number of filters for all possible combinations of page statuses and would additionally provide a overview that filters can't give. It's an information-dense (efficient) to present the information that does not conflict with the filters. It also has the benefit of greater recognizability from the way the pagelist is presented in the Index: page.

If this is too costly to implement in the widget / EIS we'll want to implement it in a new Gadget, or in one of the existing Gadgets we have that manipulate the colors and borders of the page boxes on the Index: page (and consequently we'll need some way to reliably do that).

Feel free to use the API if you need to, provided you don't add the text directly to the DOM (if you do that, it's a security issue). The reason it is marked internal only is to discourage widespread indiscriminate use of the API outside of ProofreadPage, since the API does return untrusted content that might cause a cross-site scripting vulnerability if used incorrectly.

API marked internal is a no-go for implementing a default Gadget (and the various Spanish Inquisitions at the WMF will clamp down on such usage at some point). I believe this data is already available in the imageinfo API somewhere (or at least it used to be stored with the metadata in the imageinfo table, causing it to overflow the MariaDB field limits in some cases) so this would only be a more convenient way to access the same data.

I intend to propose these changes at some point, but from my thought process at that time was that I would prefer a more accessible slightly wrong widget v/s one that I personally need to squint to see properly :(

Yeah, I'm not married to the existing colors by any means (in fact, I've considered a Gadget / local stylesheet to override it with a better color scheme; and for my transclusion checker Gadget I do use a different color scheme). But using a different color scheme is in itself an accessibility and a usability issue for large numbers of our contributors.

There's an existing task (T238382) for making the page status widget accessible in base PRP. Maybe we could kill two birds with one stone here? I'm not a fan of switching from radio buttons to a dropdown, so I think we should look for a way to preserve the good parts of the radio buttons-based interface.

EIS does use the postEdit callback, so that might be a good place to start ?

I'm not familiar with that.

I would be somewhat hesitant to add a "pre-save transform" hook here, mainly since that could lead to a bunch of race-conditions especially since EIS has no way of knowing when the script finishes without inventing a JS nightmare :(

I'm not talking about an actual high level pre-save transform feature. The Gadget (doc) just adds a .onClick() handler to the "Publish changes" and "Show changes" buttons, performs the manipulations of the text it needs, and then submits the form. It'll need to do the same when the editing is done inside the EIS UI.

I would personally refrain from manipulating it right now, Edit Recovery is coming soon, and we could potentially transform away from our current approach to one that is a wrapper around EditRecovery.

Yeah, I just need to start adapting our Gadgets and stuff to work with EIS since it's deployed already, or we're going to have a lot of confused and frustrated users with torches and pitchforks.

In T308098#9101401, @Xover wrote:
In T308098#9100738, @Soda wrote:

EIS does use the postEdit callback, so that might be a good place to start ?

I'm not familiar with that.

Oh, you mean mw.hook('postEdit')? That's misnamed; it's not post edit it's post save. We need to transform the content of e.g. #wpTextfield1 before it's saved.

In T308098#9092646, @Soda wrote:

This is the beta (to a certain extent), if stuff doesn't work, you are not forced to use it. Having a beta feature that enables a button that enables a feature isn't really a usefull way of implementing this in my opinion

@Soda : by "adding it in Beta", I meant add it in the Beta options of preferences, so that contributors could De-activate it when not interested...

During August, it is vacations in France, and many our contributors (and admins) aren't online to check what happens... Adding this tool on all wikisources at the same time when there are few people able to test and notify, without possibility to deactivate the BIg button (it is almost as ide by itself as the 3 ones previously, which gives a lot of place for accidental clicking on it)

One of our contributors reported problems with the content of the pages after a few ones (in creation) : the texte of the pages seems to .... slide, so that text of page 8 appears in page 9, and so on... this is very problematic, and I just got the same on the book I "validate" : the text from a page slid on the following page... (see image)

image.png (728×1 px, 291 KB)

Many of our contributors would like to deactivate the button, and only have it when they intend to test... and I am one of them : this not reliable as it is, and needs further reviewing and debugging - as far as I would love to work in sequence (since I edit a lot - I'm admin on frwikisource), I cannot rely on this tool AS IT IS NOW.

Please de-activate, or pass it in Beta, so that people who don't trust it can de-activate.

Hello,

There is a bug in the script, when the option for the Button is "-> next page" : I enabled it a dozens of minutes ago, and it already skipped a page, 3 times : not showing them at all. When I tried to go back, the content displayed was NOT the text of the image :(

In T308098#9092646, @Soda wrote:

This is the beta (to a certain extent), if stuff doesn't work, you are not forced to use it. Having a beta feature that enables a button that enables a feature isn't really a usefull way of implementing this in my opinion

@Soda : by "adding it in Beta", I meant add it in the Beta options of preferences, so that contributors could De-activate it when not interested...

During August, it is vacations in France, and many our contributors (and admins) aren't online to check what happens... Adding this tool on all wikisources at the same time when there are few people able to test and notify, without possibility to deactivate the BIg button (it is almost as ide by itself as the 3 ones previously, which gives a lot of place for accidental clicking on it)

One of our contributors reported problems with the content of the pages after a few ones (in creation) : the texte of the pages seems to .... slide, so that text of page 8 appears in page 9, and so on... this is very problematic, and I just got the same on the book I "validate" : the text from a page slid on the following page... (see image)

image.png (728×1 px, 291 KB)

Many of our contributors would like to deactivate the button, and only have it when they intend to test... and I am one of them : this not reliable as it is, and needs further reviewing and debugging - as far as I would love to work in sequence (since I edit a lot - I'm admin on frwikisource), I cannot rely on this tool AS IT IS NOW.

Please de-activate, or pass it in Beta, so that people who don't trust it can de-activate.

I'm currently travelling, but based on the responses here, I'll put in a patch to disable this by default (and move it to a beta feature) asap.

(see image)

Unfortunately we can’t see the image, please attach it to the task to make it visible. Thanks in advance!

Sorry @Tacsipacsi , is that OK now ? (not very used to post on this interface)

Sorry @Tacsipacsi , is that OK now ? (not very used to post on this interface)

Looks fine :)

Also, may I suggest a change of Name in French please : "Modifier les pages dans l'ordre" makes no sense in French.

It's only because I already knew @Alex_brollo's script (I tested it a little, years ago, when in was in development on itws), that I understood what it was.

Could you change Fr tab to "Modifier en série", or "Editer en série" ? or is it a parameter that can be managed on every project ?

Incidentally, judging by the volume and variety of feedback on this feature (I don't think I've seen this much engagement on a change to PRP so long as I've been active), it might be worthwhile to set up a #eis tag and / or a separate Edit-in-Sequence column on the ProofreadPage workboard so that issues can be filed as separate tasks against ProofreadPage but still grouped so you can keep track of them.

Change 951953 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[mediawiki/extensions/ProofreadPage@master] Move edit-in-sequence behind a feature flag

https://gerrit.wikimedia.org/r/951953

Change 951953 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Move edit-in-sequence behind a beta feature

https://gerrit.wikimedia.org/r/951953

Hmm. The Page Status menu item icons also do not change appearance when the menu item is disabled (i.e. the "Validated" option), making it hard to tell that the menu item is in fact disabled. This isn't helped by the low contrast change to the menu item text label: it looks very little different from a normal enabled item. Further exacerbated in a typical case of looking at it on a "Proofread" page, when the much-more contrasty menu item is between the normal items and the disabled items.

The indication for the current status in the menu is also somewhat confusing. It looks more like the mouseover hover styling than a currently-selected indicator. This should probably have a checkmark or similar distinct indication for the selected item; not have a background color change (reserve that for mouseover); and possibly make the text label bold in addition to changing the color. I'm not sure color changes for the text label are a good indication here at all since we have the color prominent in the icons too. i.e. colors are being used to indicate two different things: page status and whether a menu item is selected, increasing the cognitive load of using it.

In T308098#9121886, @Xover wrote:

Hmm. The Page Status menu item icons also do not change appearance when the menu item is disabled (i.e. the "Validated" option), making it hard to tell that the menu item is in fact disabled. This isn't helped by the low contrast change to the menu item text label: it looks very little different from a normal enabled item. Further exacerbated in a typical case of looking at it on a "Proofread" page, when the much-more contrasty menu item is between the normal items and the disabled items.

The indication for the current status in the menu is also somewhat confusing. It looks more like the mouseover hover styling than a currently-selected indicator. This should probably have a checkmark or similar distinct indication for the selected item; not have a background color change (reserve that for mouseover); and possibly make the text label bold in addition to changing the color. I'm not sure color changes for the text label are a good indication here at all since we have the color prominent in the icons too. i.e. colors are being used to indicate two different things: page status and whether a menu item is selected, increasing the cognitive load of using it.

These are the default styling used by OOUI, they aren't my favourite, but changing them is somewhat lower priority over the other more glaring bugs.

In T308098#9101401, @Xover wrote:
In T308098#9100738, @Soda wrote:

I would recommend using the PageSelection API to do this

My point was that filters, while great for other things, is not the best model for the kinds of use cases I mentioned. Color coding the page boxes in the widget would obviate the need for a large number of filters for all possible combinations of page statuses and would additionally provide a overview that filters can't give. It's an information-dense (efficient) to present the information that does not conflict with the filters. It also has the benefit of greater recognizability from the way the pagelist is presented in the Index: page.

If this is too costly to implement in the widget / EIS we'll want to implement it in a new Gadget, or in one of the existing Gadgets we have that manipulate the colors and borders of the page boxes on the Index: page (and consequently we'll need some way to reliably do that).

Combining filters is not something I have looked into (but might be a good followup).

Adding/hardcoding a multitude of colors into software is something I really do not want to implement and adding filters was a good middle ground that even if information sparse would be able to convey the same information.

Feel free to use the API if you need to, provided you don't add the text directly to the DOM (if you do that, it's a security issue). The reason it is marked internal only is to discourage widespread indiscriminate use of the API outside of ProofreadPage, since the API does return untrusted content that might cause a cross-site scripting vulnerability if used incorrectly.

API marked internal is a no-go for implementing a default Gadget (and the various Spanish Inquisitions at the WMF will clamp down on such usage at some point). I believe this data is already available in the imageinfo API somewhere (or at least it used to be stored with the metadata in the imageinfo table, causing it to overflow the MariaDB field limits in some cases) so this would only be a more convenient way to access the same data.

I talked with some people in the hackathon and there are no such plans to do this (AFAIK). If such a eventuality happens, we can look into other options in ProofreadPage

I intend to propose these changes at some point, but from my thought process at that time was that I would prefer a more accessible slightly wrong widget v/s one that I personally need to squint to see properly :(

Yeah, I'm not married to the existing colors by any means (in fact, I've considered a Gadget / local stylesheet to override it with a better color scheme; and for my transclusion checker Gadget I do use a different color scheme). But using a different color scheme is in itself an accessibility and a usability issue for large numbers of our contributors.

There's an existing task (T238382) for making the page status widget accessible in base PRP. Maybe we could kill two birds with one stone here? I'm not a fan of switching from radio buttons to a dropdown, so I think we should look for a way to preserve the good parts of the radio buttons-based interface.

I've looked at adding some kind of shortcut interface to this featureto make it more easier to use for power users. Wrt to the other task, it's on my radar and something I want to take a look at after this.

EIS does use the postEdit callback, so that might be a good place to start ?

I'm not familiar with that.

I would be somewhat hesitant to add a "pre-save transform" hook here, mainly since that could lead to a bunch of race-conditions especially since EIS has no way of knowing when the script finishes without inventing a JS nightmare :(

I'm not talking about an actual high level pre-save transform feature. The Gadget (doc) just adds a .onClick() handler to the "Publish changes" and "Show changes" buttons, performs the manipulations of the text it needs, and then submits the form. It'll need to do the same when the editing is done inside the EIS UI.

I would personally refrain from manipulating it right now, Edit Recovery is coming soon, and we could potentially transform away from our current approach to one that is a wrapper around EditRecovery.

Yeah, I just need to start adapting our Gadgets and stuff to work with EIS since it's deployed already, or we're going to have a lot of confused and frustrated users with torches and pitchforks.

Feel free to ping me on my talk page/or email regarding the scripts, will be happy to help out.

Also @Xover @Hsarrazin we are moving this to a beta feature starting this week, I've put it on next week tech nws, but it would be great if y'all could notify your respective communities.

Change 952928 had a related patch set uploaded (by Sohom Datta; author: Sohom Datta):

[operations/mediawiki-config@master] Allow loading Edit-in-Sequence as a beta feature on Wikisources

https://gerrit.wikimedia.org/r/952928

Change 952928 merged by jenkins-bot:

[operations/mediawiki-config@master] Allow loading Edit-in-Sequence as a beta feature on Wikisources

https://gerrit.wikimedia.org/r/952928

Mentioned in SAL (#wikimedia-operations) [2023-08-31T13:27:34Z] <sgimeno@deploy1002> Started scap: Backport for [[gerrit:952928|Allow loading Edit-in-Sequence as a beta feature on Wikisources (T308098)]]

Mentioned in SAL (#wikimedia-operations) [2023-08-31T13:29:11Z] <sgimeno@deploy1002> sgimeno and soda: Backport for [[gerrit:952928|Allow loading Edit-in-Sequence as a beta feature on Wikisources (T308098)]] synced to the testservers mwdebug1002.eqiad.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug2001.codfw.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Mentioned in SAL (#wikimedia-operations) [2023-08-31T13:42:34Z] <sgimeno@deploy1002> Finished scap: Backport for [[gerrit:952928|Allow loading Edit-in-Sequence as a beta feature on Wikisources (T308098)]] (duration: 15m 00s)

Mentioned in SAL (#wikimedia-operations) [2023-08-31T13:47:09Z] <sgimeno@deploy1002> Started scap: Backport for [[gerrit:952928|Allow loading Edit-in-Sequence as a beta feature on Wikisources (T308098)]]

Mentioned in SAL (#wikimedia-operations) [2023-08-31T13:48:46Z] <sgimeno@deploy1002> soda and sgimeno: Backport for [[gerrit:952928|Allow loading Edit-in-Sequence as a beta feature on Wikisources (T308098)]] synced to the testservers mwdebug2002.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug1001.eqiad.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Mentioned in SAL (#wikimedia-operations) [2023-08-31T13:56:42Z] <sgimeno@deploy1002> Finished scap: Backport for [[gerrit:952928|Allow loading Edit-in-Sequence as a beta feature on Wikisources (T308098)]] (duration: 09m 33s)

Mentioned in SAL (#wikimedia-operations) [2023-08-31T13:59:41Z] <sgimeno@deploy1002> Started scap: Backport for [[gerrit:952928|Allow loading Edit-in-Sequence as a beta feature on Wikisources (T308098)]]

Mentioned in SAL (#wikimedia-operations) [2023-08-31T14:01:21Z] <sgimeno@deploy1002> sgimeno and soda: Backport for [[gerrit:952928|Allow loading Edit-in-Sequence as a beta feature on Wikisources (T308098)]] synced to the testservers mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug2001.codfw.wmnet, mwdebug1002.eqiad.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Mentioned in SAL (#wikimedia-operations) [2023-08-31T14:07:18Z] <sgimeno@deploy1002> Finished scap: Backport for [[gerrit:952928|Allow loading Edit-in-Sequence as a beta feature on Wikisources (T308098)]] (duration: 07m 36s)