Page MenuHomePhabricator

EditRecovery could lead to users accidentally overwriting other (or their own) edits
Closed, ResolvedPublicFeature

Description

Steps to replicate the issue (include links if applicable):

  • Open a random page
  • Make a edit
  • Wait for edit to be saved via EditRecovery
  • Close the page without saving
  • As a different user (or as the same user in a different browser), open the same page
  • Make a edit
  • Save page
  • As the previous user (or using the original browser) open the same page in edit mode

What happens?:
The text shown to the user is old edit that was saved by EditRecovery. There is no indication that another edit has been made in the ensuing time period, which could lead to the user overwriting their (or a different user's edit)

What should have happened instead?:
There should be some indication that a intermediate edit has occured and the saved edit being applied is from a stale/old revision of the page.

Software version (skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Soda changed the subtype of this task from "Bug Report" to "Feature Request".Nov 16 2023, 5:31 PM

I've been thinking about this for a bit, and am I right in thinking that it's the same situation when the user didn't close the first edit window at all? i.e.:

  1. Open a random page
  2. Make a edit
  3. Wait for edit to be saved via EditRecovery
  4. Close the page without saving
  5. As a different user (or as the same user in a different browser, or just any other user?), open the same page
  6. Make a edit
  7. Save page
  8. Go back to the first edit window, and there's no indication that another edit has been made

That is, until one clicks save, at which point any edit conflict will be caught. It certainly seems like something that Edit Recovery should make more apparent, but I also wonder whether it's actually more widespread and so should be fixed elsewhere.

I think a related Wishlist proposal would probably help: T329975: Notify user when another user tries to edit an article that is being edited at the same time (although if there's to be a notification of other-edits, then that'd mean that when an edit window is re-opened the user would get two notifications, one for Edit Recovery and one about the subsequent edit).

I've been thinking about this for a bit, and am I right in thinking that it's the same situation when the user didn't close the first edit window at all? i.e.:

  1. Open a random page
  2. Make a edit
  3. Wait for edit to be saved via EditRecovery
  4. Close the page without saving
  5. As a different user (or as the same user in a different browser, or just any other user?), open the same page
  6. Make a edit
  7. Save page
  8. Go back to the first edit window, and there's no indication that another edit has been made

That is, until one clicks save, at which point any edit conflict will be caught. It certainly seems like something that Edit Recovery should make more apparent, but I also wonder whether it's actually more widespread and so should be fixed elsewhere.

I think a related Wishlist proposal would probably help: T329975: Notify user when another user tries to edit an article that is being edited at the same time (although if there's to be a notification of other-edits, then that'd mean that when an edit window is re-opened the user would get two notifications, one for Edit Recovery and one about the subsequent edit).

It is definitely part of the overarching issue, but T329975 would not help if the user isn't actively editing (since no storage of earlier edits is assumed) :)

I have personally conflicted with myself a few times while testing patches for PageTriage with a workflow similar to this:

  1. I make a edit wait for a bit and then decide to abandon the edit by navigating away and then clicking on the beforeunload dialog
  2. Come back and tag the page with a maintainenance notice
  3. Open the edit page again, and the EditRecovery dialog pops up and the tags added by PageTriage are removed.

If that last situation is true, then it could at least do a check upon opening the editor to compare base revision and warn the user if they are different.

Don't you think @Samwilson

T329975 would not help if the user isn't actively editing

You mean if the other user wasn't actively editing? That's true.

do a check upon opening the editor to compare base revision and warn the user if they are different.

It definitely could do that. It could look at the value of editRevId when loading and if it is different to the value stored it could add to the notification text and say "Your unsaved changes have been automatically recovered. Note that the page may have changed since you started editing, please 'show changes' before saving."

I think I wasn't quite grokking the scope of T329975, I was thinking that it'd alert the user to any other edit to an article, i.e. that when our user comes back to edit an article and the Edit Recovery data is loaded, there would be another thing that checks whether any other change has happened and would show a separate alert. That would mean that it'd still work even without Edit Recovery in the picture (i.e. if someone was just taking a while in editing, they'd still get notified of a change).

Change 978723 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/core@master] Edit Recovery: Add message if parent revision ID doesn't match

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

Change 978723 merged by jenkins-bot:

[mediawiki/core@master] Edit Recovery: Add message if parent revision ID doesn't match

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

The warning is added now. Is this sufficient? Or is there more we should do here?

This is what the warning looks like when you edit a page that's now got a different parent revision:

parentrev warning.png (231×282 px, 17 KB)

Sort of a nitpick, but maybe change "before saving" to "before publishing", to be consistent with the button? I recall the button was reworded out of the concern that some users were confused as to what the button was for.

Sort of a nitpick, but maybe change "before saving" to "before publishing", to be consistent with the button?

Good point. This needs to honour the $wgEditSubmitButtonLabelPublish config.

Change 1003265 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/core@master] Edit Recovery: Switch recovery message language based on $wgEditSubmitButtonLabelPublish

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

Patch for that is above. E.g. with $wgEditSubmitButtonLabelPublish = true:

Screenshot 2024-02-14 at 14-29-09 Editing Foo - Dev wiki1.png (226×284 px, 17 KB)

Change 1003265 merged by jenkins-bot:

[mediawiki/core@master] Edit Recovery: Switch recovery message language based on $wgEditSubmitButtonLabelPublish

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

I cannot get the new message to appear when doing "Show preview" or "Show changes". @Samwilson how do you trigger this?

You have to have saved a new revision of the page after you opened it for editing, something like:

  1. start editing a page;
  2. open the same page in a different browser (or private window), and save a change;
  3. close and re-open the first editing window, to get the recovery notificiation, and it should have a warning that there's been a subsequent change.

You have to have saved a new revision of the page after you opened it for editing, something like:

But after step 3, if I click "Show preview" or "Show changes" the warning does not appear.

Do you get a notification when re-opening it though, at step 3? The new warning is within the same notification toast. If you click one of the buttons at that point, it'll disappear.

Do you get a notification when re-opening it though, at step 3? The new warning is within the same notification toast. If you click one of the buttons at that point, it'll disappear.

I do. I was talking about the notification that appears on the diff or preview page when there is an intervening edit.

I was talking about the notification that appears on the diff or preview page when there is an intervening edit.

At that point, the user has already been given the warning. Maybe it should be made more visible, or shown multiple times?

@Soda What do you think of the new warning? Is it sufficient?

I also wonder if the work on notifying about concurrent editing will be of more use here? (Regardless of whether Edit Recovery is enabled.) T329975: Notify user when another user tries to edit an article that is being edited at the same time

I don't think I have anything to add here, so I will move this to Done.