paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-04 14:03:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

reload: Manually save draft and preserve id when triggering reload.

Browse Source 
We received a complaint about the generation of multiple duplicate
drafts for a single message. It was discovered that the likely cause
of this was how we were handling clients that were frequently
suspending/unsuspending, we would initiate a reload when we discovered
this, and expect the `beforeunload` handler to save the draft. This
behaved correctly, however, we would also save the compose state and
fill it in via `preserve_state` in reload.js. The important detail
here is that `preserve_state` would not encode and preserve the
`draft_id` for the current message, partly because it had no way of
knowing the `draft_id` of the draft... since we have not saved it yet,
the `beforeunload` event happens after `preserve_state`. As such,
performing any action that would trigger a draft to be saved, eg
pressing Esc to close the compose box, would save a duplicate draft of
the same message.

To resolve the above bug, we (1) ensure that we call
`drafts.update_draft()` in `preserve_state`, this returns a draft_id
to us, which we (2) ensure that we encode as part of the url and (3)
set on the `#composebox-textarea` as a `draft-id` data attribute,
which we check the next time we try to save the draft, post reload.

Note that this causes us to save the draft twice, once from
preserve_state and then again from the `beforeunload` handler, but we
do not add two drafts since the second update_draft call just edits
the timestamp because it finds the `draft-id` data attribute on the
`#composebox-textarea` set by the first call.
This commit is contained in:
YashRE42
2021-11-30 22:27:42 +05:30
committed by Tim Abbott
parent 3e853d6f8f
commit be76d7592a
2 changed files with 10 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
static/js/compose_actions.js
View File

@@ -276,6 +276,10 @@ export function start(msg_type, opts) {
compose_state.message_content(opts.content); compose_state.message_content(opts.content);
} }
if (opts.draft_id) {
$("#compose-textarea").data("draft-id", opts.draft_id);
}
compose_state.set_message_type(msg_type); compose_state.set_message_type(msg_type);
// Show either stream/topic fields or "You and" field. // Show either stream/topic fields or "You and" field.

6
static/js/reload.js
View File

@@ -6,6 +6,7 @@ import * as compose from "./compose";
import * as compose_actions from "./compose_actions"; import * as compose_actions from "./compose_actions";
import * as compose_state from "./compose_state"; import * as compose_state from "./compose_state";
import {csrf_token} from "./csrf"; import {csrf_token} from "./csrf";
import * as drafts from "./drafts";
import * as hash_util from "./hash_util"; import * as hash_util from "./hash_util";
import * as hashchange from "./hashchange"; import * as hashchange from "./hashchange";
import {localstorage} from "./localstorage"; import {localstorage} from "./localstorage";
@@ -49,6 +50,10 @@ function preserve_state(send_after_reload, save_pointer, save_narrow, save_compo
if (msg_type) { if (msg_type) {
url += "+msg=" + encodeURIComponent(compose_state.message_content()); url += "+msg=" + encodeURIComponent(compose_state.message_content());
const draft_id = drafts.update_draft();
if (draft_id) {
url += "+draft_id=" + encodeURIComponent(draft_id);
}
} }
} }
@@ -147,6 +152,7 @@ export function initialize() {
topic: topic || "", topic: topic || "",
private_message_recipient: vars.recipient || "", private_message_recipient: vars.recipient || "",
content: vars.msg || "", content: vars.msg || "",
draft_id: vars.draft_id || "",
}); });
if (send_now) { if (send_now) {
compose.finish(); compose.finish();