diff --git a/app/javascript/controllers/list_autofill_controller.js b/app/javascript/controllers/list_autofill_controller.js index 74e827e9d..d69026859 100644 --- a/app/javascript/controllers/list_autofill_controller.js +++ b/app/javascript/controllers/list_autofill_controller.js @@ -1,5 +1,15 @@ import { Controller } from '@hotwired/stimulus' +// document.execCommand is deprecated: return false instead of throwing when a +// browser has dropped it, so that callers can fall back to setRangeText. +function tryExecCommand(command, argument) { + try { + return document.execCommand(command, false, argument) + } catch { + return false + } +} + class ListAutofillHandler { constructor(inputElement, format) { this.input = inputElement @@ -36,7 +46,7 @@ class ListAutofillHandler { // Use execCommand so the edit participates in the browser's undo // stack (setRangeText and value assignments are not undoable). this.input.setSelectionRange(lineStartPos, selectionStart) - if (!document.execCommand('delete')) { + if (!tryExecCommand('delete')) { this.input.setRangeText('', lineStartPos, selectionStart, 'start') } break @@ -49,7 +59,7 @@ class ListAutofillHandler { const text = result.text this.input.addEventListener('input', (e) => { if (e.inputType !== 'insertLineBreak') return - if (!document.execCommand('insertText', false, text)) { + if (!tryExecCommand('insertText', text)) { const el = e.currentTarget const pos = el.selectionStart el.setRangeText(text, pos, pos, 'end') diff --git a/test/system/list_autofill_test.rb b/test/system/list_autofill_test.rb index 6eef8d056..e6a7cd96a 100644 --- a/test/system/list_autofill_test.rb +++ b/test/system/list_autofill_test.rb @@ -348,4 +348,79 @@ class ListAutofillSystemTest < ApplicationSystemTestCase end end end + + def test_undo_after_autofill_reverts_everything + with_settings :text_formatting => 'textile' do + visit '/projects/ecookbook/issues/new' + + within('form#issue-form') do + find('#issue_description').send_keys('* First item') + find('#issue_description').send_keys(:enter) + find('#issue_description').send_keys('Second item') + assert_equal("* First item\n* Second item", find('#issue_description').value) + + assert_equal('', undo_states(find('#issue_description')).last) + end + end + end + + def test_undo_after_marker_removal_restores_the_marker + with_settings :text_formatting => 'textile' do + visit '/projects/ecookbook/issues/new' + + within('form#issue-form') do + find('#issue_description').send_keys('* First item') + find('#issue_description').send_keys(:enter) + find('#issue_description').send_keys(:enter) + assert_equal("* First item\n", find('#issue_description').value) + + states = undo_states(find('#issue_description')) + assert_includes(states, "* First item\n* ") + assert_equal('', states.last) + end + end + end + + def test_autofill_falls_back_when_exec_command_is_unusable + stubs = { + 'missing' => 'document.execCommand = undefined;', + 'refusing' => 'document.execCommand = function() { return false; };', + 'throwing' => 'document.execCommand = function() { throw new Error("removed"); };' + } + + with_settings :text_formatting => 'textile' do + stubs.each do |name, stub| + visit '/projects/ecookbook/issues/new' + page.execute_script(stub) + + find('#issue_description').send_keys('* First item') + find('#issue_description').send_keys(:enter) + assert_equal("* First item\n* ", find('#issue_description').value, + "marker was not inserted with a #{name} execCommand") + + find('#issue_description').send_keys(:enter) + assert_equal("* First item\n", find('#issue_description').value, + "marker was not removed with a #{name} execCommand") + end + end + end + + private + + # The browser applies an undo asynchronously and groups entries its own way, + # so wait for each press to land and collect every value seen. + def undo_states(field, max: 20) + keys = [RUBY_PLATFORM.include?('darwin') ? :command : :control, 'z'] + states = [field.value] + max.times do + field.send_keys(keys) + deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + Capybara.default_max_wait_time + sleep 0.05 while field.value == states.last && + Process.clock_gettime(Process::CLOCK_MONOTONIC) < deadline + break if field.value == states.last + + states << field.value + end + states + end end