Redmine

Similar to Defect #44336: Automatic list marker insertion discards the textarea undo history and does not scroll the caret into view

Patch attached for the wiki-toolbar half of this (bold/italic/etc. from the toolbar
buttons or their keyboard shortcuts). The automatic bullet insertion mentioned in the
description is a separate code path and was fixed by the patch in #44336; the toolbar path
is untouched by it.

Where it happens¶

Toolbar edits are written by encloseSelection and encloseLineSelection in
app/assets/javascripts/jstoolbar/jstoolbar.js:

this.textarea.value = this.textarea.value.substring(0, start) + subst +
this.textarea.value.substring(end);

Assigning textarea.value programmatically resets the browser's undo stack, so a toolbar
edit both discards the history before it and leaves redo desynced.

This code is unchanged in 5.1-stable, 6.0-stable, 6.1-stable and current master, so it is
not a regression from #43095 - it is just more noticeable now that the list autofill sits
next to it. (The affected version field on this issue is empty; the behavior goes back at
least to 5.1.)

Reproduction (Chromium, 6.1.3, textile)¶

1. Open a new issue form and click into the description field.
2. Type hello.
3. Click Strong (or press Ctrl+B). The textarea holds "hello **".
4. Press Ctrl+Z ten times: the textarea still holds "hello **" - neither the inserted
   markup nor the typed text can be undone.
5. Press Ctrl+Shift+Z ten times: the textarea now holds "hello **hellohello" - redo
   replays the pre-assignment typing on top of the inserted markup.

Same in common_mark, and same for every other button (italic, underline, deleted, code,
preformatted, highlighted code, quote, bullet/numbered/task lists, headings) and for the
Ctrl+B / Ctrl+I / Ctrl+U shortcuts.

Patch¶

jstoolbar.js gains one helper:

replaceRange: function(start, end, subst) {
  if (typeof(document.execCommand) != 'function') { return false; }
  this.textarea.setSelectionRange(start, end);
  if (subst.length > 0) {
    return document.execCommand('insertText', false, subst);
  } else if (start != end) {
    return document.execCommand('delete');
  }
  return true;
},

encloseSelection and encloseLineSelection call it instead of assigning
textarea.value, and keep the assignment as a fallback where execCommand is
unavailable. Caret handling is unchanged: after insertText the caret already sits at
start + subst.length, so only the "collapse between the markers" case still needs an
explicit setSelectionRange. The patch applies cleanly to 6.1-stable and to current
master.

Verified (6.1.3, headless Chromium driven by Playwright, textile and common_mark)¶

• inserted text and caret position are identical to the current behavior for every toolbar
  button and shortcut - 17 scenarios in textile, 18 in common_mark - including selection
  wrapping and the highlighted-code language menu
• Ctrl+Z reverts a toolbar insertion in one step and then continues into the text typed
  before it; Ctrl+Shift+Z restores exactly the post-button text
• the values asserted in test/system/keyboard_shortcuts_test.rb come out unchanged
  (checked in the browser rather than by running the suite): ** / ++ / __ for textile,
  **** / <u></u> / ** for common_mark
• scroll position is unchanged when a button is pressed with the caret at the bottom of a
  scrolled textarea
• execCommand is deprecated but universally supported for insertText/delete in
  textareas, and core already relies on it (copy-to-clipboard)

One side benefit: execCommand edits fire an input event, which a value assignment does
not, so anything listening for editor changes (draft autosave, plugins) finally sees text
inserted by a toolbar button.