From 0bb0061915adaf1dd72fb6d048c076ce2fdaf5ab Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Thu, 23 Jan 2025 10:50:00 +0100 Subject: [PATCH] LibWeb: Fire `input` events in `.execCommand()` We do not fire `beforeinput` events since other browsers do not seem to do so either. The spec asks us to check whether a command's action modified the DOM tree. This means adding or removing nodes and attributes, or changing character data anywhere in the tree. We have `Document::dom_tree_version()` for node updates, but for character data a new version number is introduced that allows us to easily keep track of any text changes in the entire tree. --- Libraries/LibWeb/DOM/CharacterData.cpp | 1 + Libraries/LibWeb/DOM/Document.h | 6 + Libraries/LibWeb/DOM/MutationObserver.h | 1 - Libraries/LibWeb/Editing/Commands.cpp | 21 ++++ Libraries/LibWeb/Editing/Commands.h | 3 + Libraries/LibWeb/Editing/ExecCommand.cpp | 115 +++++++++++------- .../Editing/execCommand-insertText.txt | 1 + .../input/Editing/execCommand-insertText.html | 1 + 8 files changed, 106 insertions(+), 43 deletions(-) diff --git a/Libraries/LibWeb/DOM/CharacterData.cpp b/Libraries/LibWeb/DOM/CharacterData.cpp index 66d2a91c04a7..ceab2f30316e 100644 --- a/Libraries/LibWeb/DOM/CharacterData.cpp +++ b/Libraries/LibWeb/DOM/CharacterData.cpp @@ -135,6 +135,7 @@ WebIDL::ExceptionOr CharacterData::replace_data(size_t offset, size_t coun static_cast(*layout_node).invalidate_text_for_rendering(); document().set_needs_layout(); + document().bump_character_data_version(); if (m_grapheme_segmenter) m_grapheme_segmenter->set_segmented_text(m_data); diff --git a/Libraries/LibWeb/DOM/Document.h b/Libraries/LibWeb/DOM/Document.h index 8f906c911321..62d9fa17c1e1 100644 --- a/Libraries/LibWeb/DOM/Document.h +++ b/Libraries/LibWeb/DOM/Document.h @@ -113,6 +113,11 @@ class Document u64 dom_tree_version() const { return m_dom_tree_version; } void bump_dom_tree_version() { ++m_dom_tree_version; } + // AD-HOC: This number increments whenever CharacterData is modified in the document. It is used together with + // dom_tree_version() to understand whether either the DOM tree structure or contents were changed. + u64 character_data_version() const { return m_character_data_version; } + void bump_character_data_version() { ++m_character_data_version; } + WebIDL::ExceptionOr populate_with_html_head_and_body(); GC::Ptr get_selection() const; @@ -1067,6 +1072,7 @@ class Document Optional m_last_modified; u64 m_dom_tree_version { 0 }; + u64 m_character_data_version { 0 }; // https://drafts.csswg.org/css-position-4/#document-top-layer // Documents have a top layer, an ordered set containing elements from the document. diff --git a/Libraries/LibWeb/DOM/MutationObserver.h b/Libraries/LibWeb/DOM/MutationObserver.h index a00762d53828..570a6c48ce97 100644 --- a/Libraries/LibWeb/DOM/MutationObserver.h +++ b/Libraries/LibWeb/DOM/MutationObserver.h @@ -7,7 +7,6 @@ #pragma once -#include #include #include #include diff --git a/Libraries/LibWeb/Editing/Commands.cpp b/Libraries/LibWeb/Editing/Commands.cpp index 58e550c24c42..6a731d9c86c3 100644 --- a/Libraries/LibWeb/Editing/Commands.cpp +++ b/Libraries/LibWeb/Editing/Commands.cpp @@ -2464,6 +2464,7 @@ static Array const commands { .command = CommandNames::backColor, .action = command_back_color_action, .relevant_css_property = CSS::PropertyID::BackgroundColor, + .mapped_value = "formatBackColor"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-bold-command CommandDefinition { @@ -2471,17 +2472,20 @@ static Array const commands { .action = command_bold_action, .relevant_css_property = CSS::PropertyID::FontWeight, .inline_activated_values = { "bold"sv, "600"sv, "700"sv, "800"sv, "900"sv }, + .mapped_value = "formatBold"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-createlink-command CommandDefinition { .command = CommandNames::createLink, .action = command_create_link_action, + .mapped_value = "insertLink"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-delete-command CommandDefinition { .command = CommandNames::delete_, .action = command_delete_action, .preserves_overrides = true, + .mapped_value = "deleteContentBackward"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-defaultparagraphseparator-command CommandDefinition { @@ -2494,6 +2498,7 @@ static Array const commands { .command = CommandNames::fontName, .action = command_font_name_action, .relevant_css_property = CSS::PropertyID::FontFamily, + .mapped_value = "formatFontName"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-fontsize-command CommandDefinition { @@ -2507,6 +2512,7 @@ static Array const commands { .command = CommandNames::foreColor, .action = command_fore_color_action, .relevant_css_property = CSS::PropertyID::Color, + .mapped_value = "formatFontColor"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-formatblock-command CommandDefinition { @@ -2521,6 +2527,7 @@ static Array const commands { .command = CommandNames::forwardDelete, .action = command_forward_delete_action, .preserves_overrides = true, + .mapped_value = "deleteContentForward"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-hilitecolor-command CommandDefinition { @@ -2533,12 +2540,14 @@ static Array const commands { .command = CommandNames::indent, .action = command_indent_action, .preserves_overrides = true, + .mapped_value = "formatIndent"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-inserthorizontalrule-command CommandDefinition { .command = CommandNames::insertHorizontalRule, .action = command_insert_horizontal_rule_action, .preserves_overrides = true, + .mapped_value = "insertHorizontalRule"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-inserthtml-command CommandDefinition { @@ -2557,6 +2566,7 @@ static Array const commands { .command = CommandNames::insertLineBreak, .action = command_insert_linebreak_action, .preserves_overrides = true, + .mapped_value = "insertLineBreak"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-insertorderedlist-command CommandDefinition { @@ -2565,17 +2575,20 @@ static Array const commands { .indeterminate = command_insert_ordered_list_indeterminate, .state = command_insert_ordered_list_state, .preserves_overrides = true, + .mapped_value = "insertOrderedList"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-insertparagraph-command CommandDefinition { .command = CommandNames::insertParagraph, .action = command_insert_paragraph_action, .preserves_overrides = true, + .mapped_value = "insertParagraph"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-inserttext-command CommandDefinition { .command = CommandNames::insertText, .action = command_insert_text_action, + .mapped_value = "insertText"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-insertunorderedlist-command CommandDefinition { @@ -2584,6 +2597,7 @@ static Array const commands { .indeterminate = command_insert_unordered_list_indeterminate, .state = command_insert_unordered_list_state, .preserves_overrides = true, + .mapped_value = "insertUnorderedList"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-italic-command CommandDefinition { @@ -2600,6 +2614,7 @@ static Array const commands { .state = command_justify_center_state, .value = command_justify_center_value, .preserves_overrides = true, + .mapped_value = "formatJustifyCenter"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-justifyfull-command CommandDefinition { @@ -2609,6 +2624,7 @@ static Array const commands { .state = command_justify_full_state, .value = command_justify_full_value, .preserves_overrides = true, + .mapped_value = "formatJustifyFull"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-justifyleft-command CommandDefinition { @@ -2618,6 +2634,7 @@ static Array const commands { .state = command_justify_left_state, .value = command_justify_left_value, .preserves_overrides = true, + .mapped_value = "formatJustifyLeft"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-justifyright-command CommandDefinition { @@ -2627,12 +2644,14 @@ static Array const commands { .state = command_justify_right_state, .value = command_justify_right_value, .preserves_overrides = true, + .mapped_value = "formatJustifyRight"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-outdent-command CommandDefinition { .command = CommandNames::outdent, .action = command_outdent_action, .preserves_overrides = true, + .mapped_value = "formatOutdent"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-removeformat-command CommandDefinition { @@ -2649,6 +2668,7 @@ static Array const commands { .command = CommandNames::strikethrough, .action = command_strikethrough_action, .inline_activated_values = { "line-through"sv }, + .mapped_value = "formatStrikeThrough"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-stylewithcss-command CommandDefinition { @@ -2669,6 +2689,7 @@ static Array const commands { .action = command_superscript_action, .indeterminate = command_superscript_indeterminate, .inline_activated_values = { "superscript"sv }, + .mapped_value = "formatSuperscript"_fly_string, }, // https://w3c.github.io/editing/docs/execCommand/#the-underline-command CommandDefinition { diff --git a/Libraries/LibWeb/Editing/Commands.h b/Libraries/LibWeb/Editing/Commands.h index 2c4e15da17fb..c39d0f510da9 100644 --- a/Libraries/LibWeb/Editing/Commands.h +++ b/Libraries/LibWeb/Editing/Commands.h @@ -24,6 +24,9 @@ struct CommandDefinition { // https://w3c.github.io/editing/docs/execCommand/#inline-command-activated-values Vector inline_activated_values {}; + + // https://w3c.github.io/editing/docs/execCommand/#dfn-map-an-edit-command-to-input-type-value + FlyString mapped_value {}; }; Optional find_command_definition(FlyString const&); diff --git a/Libraries/LibWeb/Editing/ExecCommand.cpp b/Libraries/LibWeb/Editing/ExecCommand.cpp index aa95b7ddf962..b1c68f4e38a4 100644 --- a/Libraries/LibWeb/Editing/ExecCommand.cpp +++ b/Libraries/LibWeb/Editing/ExecCommand.cpp @@ -5,11 +5,13 @@ */ #include +#include #include #include #include #include #include +#include namespace Web::DOM { @@ -37,46 +39,52 @@ WebIDL::ExceptionOr Document::exec_command(FlyString const& command, [[may // 4. If command is not in the Miscellaneous commands section: // - // We don't fire events for copy/cut/paste/undo/redo/selectAll because they should all have - // their own events. We don't fire events for styleWithCSS/useCSS because it's not obvious - // where to fire them, or why anyone would want them. We don't fire events for unsupported - // commands, because then if they became supported and were classified with the miscellaneous - // events, we'd have to stop firing events for consistency's sake. + // We don't fire events for copy/cut/paste/undo/redo/selectAll because they should all have their own events. We + // don't fire events for styleWithCSS/useCSS because it's not obvious where to fire them, or why anyone would + // want them. We don't fire events for unsupported commands, because then if they became supported and were + // classified with the miscellaneous events, we'd have to stop firing events for consistency's sake. // // AD-HOC: The defaultParagraphSeparator command is also in the Miscellaneous commands section - if (command != Editing::CommandNames::defaultParagraphSeparator - && command != Editing::CommandNames::redo - && command != Editing::CommandNames::selectAll - && command != Editing::CommandNames::styleWithCSS - && command != Editing::CommandNames::undo - && command != Editing::CommandNames::useCSS) { - // FIXME: 1. Let affected editing host be the editing host that is an inclusive ancestor of the - // active range's start node and end node, and is not the ancestor of any editing host - // that is an inclusive ancestor of the active range's start node and end node. - - // FIXME: 2. Fire an event named "beforeinput" at affected editing host using InputEvent, with its + GC::Ptr affected_editing_host; + if (!command.is_one_of(Editing::CommandNames::copy, Editing::CommandNames::cut, + Editing::CommandNames::defaultParagraphSeparator, Editing::CommandNames::paste, Editing::CommandNames::redo, + Editing::CommandNames::selectAll, Editing::CommandNames::styleWithCSS, Editing::CommandNames::undo, + Editing::CommandNames::useCSS)) { + // 1. Let affected editing host be the editing host that is an inclusive ancestor of the active range's start + // node and end node, and is not the ancestor of any editing host that is an inclusive ancestor of the active + // range's start node and end node. + // + // NOTE: Because either the start or end node of the range could be inside an editing host that is part of the + // other node's editing host, we can probe both and see if either one is the other's ancestor. + // NOTE: We can reuse Editing::editing_host_of_node() here since query_command_enabled() above already checked + // that both the start and end nodes are either editable or an editing host. + auto range = Editing::active_range(*this); + auto& start_node_editing_host = *Editing::editing_host_of_node(range->start_container()); + auto& end_node_editing_host = *Editing::editing_host_of_node(range->end_container()); + affected_editing_host = start_node_editing_host.is_ancestor_of(end_node_editing_host) + ? end_node_editing_host + : start_node_editing_host; + + // 2. Fire an event named "beforeinput" at affected editing host using InputEvent, with its // bubbles and cancelable attributes initialized to true, and its data attribute // initialized to null. - - // FIXME: 3. If the value returned by the previous step is false, return false. - + // 3. If the value returned by the previous step is false, return false. // 4. If command is not enabled, return false. // - // We have to check again whether the command is enabled, because the beforeinput handler - // might have done something annoying like getSelection().removeAllRanges(). - if (!MUST(query_command_enabled(command))) - return false; - - // FIXME: 5. Let affected editing host be the editing host that is an inclusive ancestor of the - // active range's start node and end node, and is not the ancestor of any editing host - // that is an inclusive ancestor of the active range's start node and end node. + // We have to check again whether the command is enabled, because the beforeinput handler might have done + // something annoying like getSelection().removeAllRanges(). + // 5. Let affected editing host be the editing host that is an inclusive ancestor of the active range's start + // node and end node, and is not the ancestor of any editing host that is an inclusive ancestor of the active + // range's start node and end node. // - // This new affected editing host is what we'll fire the input event at in a couple of - // lines. We want to compute it beforehand just to be safe: bugs in the command action - // might remove the selection or something bad like that, and we don't want to have to - // handle it later. We recompute it after the beforeinput event is handled so that if the - // handler moves the selection to some other editing host, the input event will be fired - // at the editing host that was actually affected. + // This new affected editing host is what we'll fire the input event at in a couple of lines. We want to + // compute it beforehand just to be safe: bugs in the command action might remove the selection or something + // bad like that, and we don't want to have to handle it later. We recompute it after the beforeinput event + // is handled so that if the handler moves the selection to some other editing host, the input event will be + // fired at the editing host that was actually affected. + + // AD-HOC: No, we don't. Neither Chrome nor Firefox fire the "beforeinput" event for execCommand(). This is an + // open discussion for the spec: https://github.com/w3c/editing/issues/200 } // https://w3c.github.io/editing/docs/execCommand/#preserves-overrides @@ -88,6 +96,10 @@ WebIDL::ExceptionOr Document::exec_command(FlyString const& command, [[may if (command_definition.preserves_overrides) overrides = Editing::record_current_overrides(*this); + // NOTE: Step 7 below asks us whether the DOM tree was modified, so keep track of the document versions. + auto old_dom_tree_version = dom_tree_version(); + auto old_character_data_version = character_data_version(); + // 5. Take the action for command, passing value to the instructions as an argument. auto command_result = command_definition.action(*this, value); @@ -101,10 +113,19 @@ WebIDL::ExceptionOr Document::exec_command(FlyString const& command, [[may if (!command_result) return false; - // FIXME: 7. If the action modified DOM tree, then fire an event named "input" at affected editing host - // using InputEvent, with its isTrusted and bubbles attributes initialized to true, inputType - // attribute initialized to the mapped value of command, and its data attribute initialized - // to null. + // 7. If the action modified DOM tree, then fire an event named "input" at affected editing host using InputEvent, + // with its isTrusted and bubbles attributes initialized to true, inputType attribute initialized to the mapped + // value of command, and its data attribute initialized to null. + bool tree_was_modified = dom_tree_version() != old_dom_tree_version + || character_data_version() != old_character_data_version; + if (tree_was_modified && affected_editing_host) { + UIEvents::InputEventInit event_init {}; + event_init.bubbles = true; + event_init.input_type = command_definition.mapped_value; + auto event = realm().create(realm(), HTML::EventNames::input, event_init); + event->set_is_trusted(true); + affected_editing_host->dispatch_event(event); + } // 8. Return true. return true; @@ -148,7 +169,7 @@ WebIDL::ExceptionOr Document::query_command_enabled(FlyString const& comma return false; // FIXME: the editing host of its start node is not an EditContext editing host, - auto start_node_editing_host = Editing::editing_host_of_node(start_node); + [[maybe_unused]] auto start_node_editing_host = Editing::editing_host_of_node(start_node); // its end node is either editable or an editing host, auto& end_node = *active_range->end_container(); @@ -156,13 +177,23 @@ WebIDL::ExceptionOr Document::query_command_enabled(FlyString const& comma return false; // FIXME: the editing host of its end node is not an EditContext editing host, - - // FIXME: and there is some editing host that is an inclusive ancestor of both its start node and its - // end node. + [[maybe_unused]] auto end_node_editing_host = Editing::editing_host_of_node(end_node); + + // and there is some editing host that is an inclusive ancestor of both its start node and its end node. + GC::Ptr inclusive_ancestor_editing_host; + start_node->for_each_inclusive_ancestor([&](GC::Ref ancestor) { + if (ancestor->is_editing_host() && ancestor->is_inclusive_ancestor_of(end_node)) { + inclusive_ancestor_editing_host = ancestor; + return IterationDecision::Break; + } + return IterationDecision::Continue; + }); + if (!inclusive_ancestor_editing_host) + return false; // NOTE: Commands can define additional conditions for being enabled, and currently the only condition mentioned in // the spec is that certain commands must not be enabled if the editing host is in the plaintext-only state. - if (auto const* html_element = as_if(start_node_editing_host.ptr()); html_element + if (auto const* html_element = as_if(inclusive_ancestor_editing_host.ptr()); html_element && html_element->content_editable_state() == HTML::ContentEditableState::PlaintextOnly && command.is_one_of( Editing::CommandNames::backColor, diff --git a/Tests/LibWeb/Text/expected/Editing/execCommand-insertText.txt b/Tests/LibWeb/Text/expected/Editing/execCommand-insertText.txt index 627d17b106ac..afc0d789a84d 100644 --- a/Tests/LibWeb/Text/expected/Editing/execCommand-insertText.txt +++ b/Tests/LibWeb/Text/expected/Editing/execCommand-insertText.txt @@ -1 +1,2 @@ +input triggered foobazbar diff --git a/Tests/LibWeb/Text/input/Editing/execCommand-insertText.html b/Tests/LibWeb/Text/input/Editing/execCommand-insertText.html index 1ac96bb2c298..e5c1e0c78331 100644 --- a/Tests/LibWeb/Text/input/Editing/execCommand-insertText.html +++ b/Tests/LibWeb/Text/input/Editing/execCommand-insertText.html @@ -3,6 +3,7 @@