I think it's a race, roughly:

• You type.
• We queue a "save a draft" request for like 1s after you stop typing.
• You stop typing, but then click "Submit" in less than 1s.
• Your submit fires and clears the draft.
• Meanwhile, the "save a draft" request fires and saves the draft back over the cleared draft.
• Page reloads.

Some possible attacks:

• Stop the "draft" request from firing when we send a submit request, to reduce the race window.
• Version the drafts? Kind of messy but not too unreasonable.
• Put a "don't write any drafts for 10s" row into the draft table after clearing a draft? This is sort of like ghetto versioning.

I think the first approach seems to be reasonable and probably not too difficult. Not knowing how the underlying (JavaScript) code works here, I feel as though if the "save draft" action is queued at some time X and the comment is submitted at some time X+Y (Y > 0) then there is no reason to store the draft.

◀ Merged tasks: T5592.

I think we can do the versioning fairly cleanly:

• Drafts get a version column, so they have <authorPHID, objectPHID / draftKey, version, text, attributes, dateCreated> or similar.
• We write the current draft version into the comment form.
• The draft update request includes the version.
• If the request version is not the same as the database version, drop the request. Otherwise, update the text/attributes.
• When clearing a draft, bump the version and wipe the text.
• In the GC cycle, purge empty drafts that are more than an hour (or whatever) old.

This has soft dependencies on T4896, T5245 and T5604, since it would be nice to unify the draft code at the same time as we do this. I think there might be a task about storing comment state in drafts more consistently somewhere, too (we store it in Differential, but not other applications).

Support Impact This is broken.

T9132 has a tangential interaction here: Differential saves action state, as well as comment state, and it would be nice to do this generally. After T9132, any generalization here can reasonably also persist action state in a general way.

This is fairly straightforward but blocked on T9132, which is prioritized elsewhere. This is likely to be completed adjacently with T9132 anyway.

I've marked D14637 as fixing this.

That comes with a caveat: it only fixes it only once applications move to EditEngine (new tech out of T9132). Currently, only Paste and Owners use it (and Owners doesn't have comments), so this won't fix Maniphest yet (which is presumably where most users are seeing this). Maniphest should move over in the next week or so, if things go well.

Broadly, if comments have the new "stacked action" UI (an "Add Action..." dropdown, allowing you to take multiple actions at once), the expectation is that they support the new drafts too. This includes Maniphest at HEAD.