Is 30 days appropriate here?

I would recommend implementing a generic expiry API, like we did with localStorage in T121646.

Yes, definitely a good idea. I've been investigating doing that, and it has been a bit of a trade off between building something now for this use case, and building a framework that could work for future use cases as well (without actually knowing what those are, at the moment — which is why I've hesitated so far).

We'll look at moving the generic bits of the storage layer to a central place e.g. mediawiki.indexedDB. mw.storage has an optional expiry, but we could make it mandatory probably.

mw.storage has an optional expiry, but we could make it mandatory probably.

Yes - it's only optional for backwards compatibility.

Change 958986 had a related patch set uploaded (by Samtar; author: Samtar):

[mediawiki/core@master] editRecovery: Delete edit recovery data after 30 days

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

Change 958986 merged by jenkins-bot:

[mediawiki/core@master] editRecovery: Delete edit recovery data after 30 days

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

@Samwilson @TheresNoTime I have created https://phabricator.wikimedia.org/T348593 to have it configurable in LocalSettings.php since this expiration date is hard coded. I did change it to 1 day so I should be able to test tomorrow afternoon, though is the key correct for the expiryDate?

Status: Pending
Environment: Local: 1.42.0-alpha (a985465) 10:25, 10 October 2023
OS: macOS Sonoma 14.0
Browser: Chrome 117
Device: MBA M2
Emulated Device:: n/a
Test Links:
http://localhost:8080/w/index.php?title=Harry_potter&action=edit

❓AC1: https://phabricator.wikimedia.org/T341956

@GMikesell-WMF Thanks! That's a good idea to make it configurable.

The key is correct for expiryDate, it's the unix timestamp of the expiry time.

@Samwilson @TheresNoTime The data did not get removed as seen in the gif. I will move this to Needs attention for now. I will also set up another test article just so I have two, just in case. They both now have an expiry date of 1697180400 which is set up for today at midnight in case you figure out why it's not deleting the data in the indexDB.

Status: ❌ FAIL
Environment: Local: 1.42.0-alpha (a985465) 10:25, 10 October 2023
OS: macOS Sonoma 14.0
Browser: Chrome 117
Device: MBA M2
Emulated Device:: n/a
Test Links:
http://localhost:8080/w/index.php?title=Harry_potter&action=edit
http://localhost:8080/w/index.php?title=Beer&action=edit

❌AC1: https://phabricator.wikimedia.org/T345315

I set up another article to test as seen in the screenshot.

In T341956#9240984, @GMikesell-WMF wrote:

I did change it to 1 day so I should be able to test tomorrow afternoon

If you move your system clock forward a day this should have the same effect.

In T341956#9249168, @Esanders wrote:

In T341956#9240984, @GMikesell-WMF wrote:

I did change it to 1 day so I should be able to test tomorrow afternoon

If you move your system clock forward a day this should have the same effect.

That's also true, thanks!

It looks like after closing the tab and opening up a new one, it looks like it just moves the expiryDate a day after again besides deleting it when comparing this screenshot and last.

Could you try again now that T348593: Edit Recovery: Configuration in LocalSettings.php for expiration time for Data removal in indexedDB is merged?

See https://phabricator.wikimedia.org/T348593#9274266

Latest patch for T348593#9402563 should resolve these issues and unblock QA for this :-) 🤞

Without understanding the context fully, it seems odd that we'd keep edit recovery for 30 days. How realistic is it for a user to write edits, go on vacation, and come back and see their unsaved edits. Are we incurring significant tech debt or effort to keep this data for 30 days? How might this change if the rule were 30 minutes, 1 hour, 24 hours?

It's a good question. I think basically we started with the idea that we're not allowed to store for more than 90 days, and so somewhat arbitrarily chose 30 days as a good time length. It's governed by a configuration variable $wgEditRecoveryExpiry and so can easily be changed at any time without a problem (although existing items' expiry dates won't be updated), and there's no code changes that we'd want to make if it was 1 day or 90.

I think that at least 7 days would be good. Personally, in Phabricator (where there's a similar feature) I often come back weeks later and am glad to find my in-progress words.

I have tested that Edit Recovery data is expired in WikiEditor on Firefox and Chromium, CodeMirror on Chromium and CodeEditor on Firefox. For each of these three editors, I tested expiry in the case of editing, viewing preview and viewing changes, for both editing a whole page and editing a section.

I also tested for each editor that while undoing an edit or viewing an old edit data was not expired, because in these cases Edit Recovery is disabled (see T347867 and T347869). The only exception is when viewing the preview or changes for an undo, where Edit Recovery is enabled and data will be expired. This bug has been reported in T347869 and will be fixed there.

I couldn't test on Safari as I can only test this change locally and I don't have access to Safari locally.

In T341956#9471565, @JWheeler-WMF wrote:

Without understanding the context fully, it seems odd that we'd keep edit recovery for 30 days. How realistic is it for a user to write edits, go on vacation, and come back and see their unsaved edits. Are we incurring significant tech debt or effort to keep this data for 30 days? How might this change if the rule were 30 minutes, 1 hour, 24 hours?

I believe there is a limit to the amount of data a browser will store in IndexedDB (see https://developer.mozilla.org/en-US/docs/Web/API/Storage_API/Storage_quotas_and_eviction_criteria). I don't know how likely users are to hit that limit. However, I have not yet tested what happens when they do.

Test environment: local docker MediaWiki 1.42.0-alpha (9e9d76a) 05:04, 22 January 2024.

Ok. 30 days sounds reasonable so long as there isn't additional tech debt.