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

[mediawiki/core@master] Edit Recovery: Don't load when undoing edits

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

In T347869#9252400, @Samwilson wrote:

In addition to not loading Edit Recovery when undoing edits, I wonder if we would want to also delete any edit recovery data for the page — because if someone's undoing to an earlier revision it's pretty likely that they're not worried about any work in progress they have for that page. Although I guess they could have started editing, hit a conflict, then gone to undo the conflict so that they can save without any issues?

Immediately, or when they save the edit?

Yeah good point, it'd have to be when they save; wouldn't want just clicking on an undo link to result in the current saved data to be deleted, I imagine.

Change 965930 merged by jenkins-bot:

[mediawiki/core@master] Edit Recovery: Don't load when undoing edits

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

After more thought, I think we should postpone deleting the data until after T342721, because that's going to make it easier to discard work-in-progress and we'll get a better idea of how people want to use this when undoing (and other things).

@Samwilson Edit Recovery is on during edit source but is disabled during Undo and with "Show Changes" and "Show Preview" from the screenshots below. Before I move this to Done, what do you think of the possible issue section?

Status: ✅PASS
Environment: Beta: 1.42.0-alpha (5a7226f)
OS: macOS Sonoma 14.0
Browser: Chrome 118, Firefox 118, Safari 17.0
Skins. Vector 2022, 2010, Minerva, Monobook, Timeless
Device: MBA M2
Emulated Device:: n/a
Test Links:
Edit Recovery is Enabled: https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Dingo&action=edit
Edit Recovery is Disabled: https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Dingo&action=edit&undoafter=605824&undo=605825

✅AC1: https://phabricator.wikimedia.org/T347869

Edit Recovery Enabled	Show Changes- ER disabled	Show preview- ER disabled

Safari	Firefox	Timeless (Same as other Skins)

Possible issue: If you click on "Discard Changes" from the Changes Recovered pop-up, it goes to the end of the article. Shouldn't it start at the beginning?

Possible issue: If you click on "Discard Changes" from the Changes Recovered pop-up, it goes to the end of the article. Shouldn't it start at the beginning?

Good point! It should ideally keep the same scroll position. I'll fix this in T342721 (as that's where that bug was introduced).

@Samwilson Great thanks!

I also noticed another issue just right now when you click on the next undo as seen in the .webm. It seems like it works on the most recent undo, in which the edit recovery is disabled, but if you go to the next undo, edit recovery is enabled again. Did you want me to create another task or will you just work off of this one?

@Samwilson I notice that Edit Recovery data is cleared after saving/publishing an undo. Is this what we want?

In T347869#9435172, @dom_walden wrote:

@Samwilson I notice that Edit Recovery data is cleared after saving/publishing an undo. Is this what we want?

No, that's a good point. The fix that @TheresNoTime is working on for T352028 is going to set a localStorage value when loading Edit Recovery, and it seems like we need to do something similar to fix this, so that the postEdit code isn't run when Edit Recovery wasn't used before the edit that's just happened.

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

[mediawiki/core@master] Edit Recovery: Improve determination of undo state

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

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

[mediawiki/core@master] Edit Recovery: Only delete in postEdit if data was saved

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

Change 987943 merged by jenkins-bot:

[mediawiki/core@master] Edit Recovery: Only delete in postEdit if data was saved

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

Change 987885 merged by jenkins-bot:

[mediawiki/core@master] Edit Recovery: Improve determination of undo state

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

@Samwilson I am finding that if show preview or show changes while on an undo page, Edit Recovery is enabled again. Could you check you are seeing the same thing?

@Samwilson The previous issues that were mentioned before are now resolved. Can you please review the last two issues that Dom found that I confirmed as seen in the .webm below for AC3 and AC4? Thanks!

Status: ❌ FAIL
Environment: Beta: 1.42.0-alpha (2f8e379)
OS: macOS Sonoma 14.2.1
Browser: Chrome 120
Skins. Vector 2022
Device: MBA M2
Emulated Device:: n/a
Test Links:
https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Dingo&action=edit

✅AC1: "Discard Changes" Scroll- now goes to the top

If you click on "Discard Changes" from the Changes Recovered pop-up, it goes to the end of the article. Shouldn't it start at the beginning?

✅AC2: Edit recovery is disabled for the following undo

I also noticed another issue just right now when you click on the next undo as seen in the .webm. It seems like it works on the most recent undo, in which the edit recovery is disabled, but if you go to the next undo, edit recovery is enabled again. Did you want me to create another task or will you just work off of this one?

❌AC3: Edit Recovery data is cleared after saving/publishing an undo- Still clears the Edit Recovery data

In T347869#9436995, @Samwilson wrote:

In T347869#9435172, @dom_walden wrote:

@Samwilson I notice that Edit Recovery data is cleared after saving/publishing an undo. Is this what we want?

No, that's a good point. The fix that @TheresNoTime is working on for T352028 is going to set a localStorage value when loading Edit Recovery, and it seems like we need to do something similar to fix this, so that the postEdit code isn't run when Edit Recovery wasn't used before the edit that's just happened.

❌AC4: Edit Recovery is enable again from an undo when you click on "Show Preview" or "Show Changes"- Edit Recovery is enabled again

@Samwilson I am finding that if show preview or show changes while on an undo page, Edit Recovery is enabled again. Could you check you are seeing the same thing?

❌AC3: Edit Recovery data is cleared after saving/publishing an undo- Still clears the Edit Recovery data

Good catch. It looks like this only happens if you do the undo within 5 minutes of editing, because we set a session flag during editing but then only clear it in postedit or while editing — so it's not cleared when loading the undo form.

I'm making a patch now to clear the flag any time Edit Recovery isn't loaded in an edit form.

❌AC4: Edit Recovery is enable again from an undo when you click on "Show Preview" or "Show Changes"- Edit Recovery is enabled again

I'll revisit how it's checking whether it's on an undo form or not. Checking the query string parameter is not sufficient.

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

[mediawiki/core@master] Edit Recovery: Delete the data-saved sessionStorage flag

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

Change 992271 merged by jenkins-bot:

[mediawiki/core@master] Edit Recovery: Delete the data-saved sessionStorage flag

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

@Samwilson Everything seems to pass now but AC5, Edit Recovery is now enabled again after the first undo as seen in the .webm.

Status: ✅PASS
Environment: Beta: 1.42.0-alpha (a88a76c)
OS: macOS Sonoma 14.2.1
Browser: Chrome 121, Firefox 123, Safari 17.3, Edge 121
Skins. Vector 2022, Vector 2010, Minerva, Monobook, Timeless
Device: MBA M2
Emulated Device:: n/a
Test Links:
https://en.m.wikipedia.beta.wmflabs.org/wiki/Dog#
https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Corsac_fox&action=edit

✅AC1: "Discard Changes" Scroll- now goes to the top - https://phabricator.wikimedia.org/T347869#9467167
✅AC2: Edit recovery is disabled for the following undo - https://phabricator.wikimedia.org/T347869#9467167

In T347869#9471799, @Samwilson wrote:

❌AC3: Edit Recovery data is cleared after saving/publishing an undo- Still clears the Edit Recovery data

Good catch. It looks like this only happens if you do the undo within 5 minutes of editing, because we set a session flag during editing but then only clear it in postedit or while editing — so it's not cleared when loading the undo form.
I'm making a patch now to clear the flag any time Edit Recovery isn't loaded in an edit form.

✅ AC3: Edit Recovery data is cleared after saving/publishing an undo

❌AC4: Edit Recovery is enable again from an undo when you click on "Show Preview" or "Show Changes"- Edit Recovery is enabled again

I'll revisit how it's checking whether it's on an undo form or not. Checking the query string parameter is not sufficient.

✅ AC4: Edit Recovery is enable again from an undo when you click on "Show Preview" or "Show Changes"

❌AC5: Edit Recovery is enabled after the first undo.