From a0b0e91d4fb87ed8146fe3237bce26289882ee7a Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Tue, 21 Jan 2025 10:13:14 +0000 Subject: [PATCH] LibWeb: Disallow Editing API calls on non-HTML documents This is not directly mentioned in the Editing API spec, but all major browsers do this and there is a WPT for this behavior. --- Libraries/LibWeb/DOM/Document.h | 12 ++--- Libraries/LibWeb/Editing/Commands.cpp | 12 ++--- Libraries/LibWeb/Editing/ExecCommand.cpp | 48 ++++++++++++++----- .../LibWeb/Editing/Internal/Algorithms.cpp | 8 ++-- .../Editing/execCommand-selectAll.txt | 2 +- .../editing/other/non-html-document.txt | 6 +++ .../input/Editing/execCommand-selectAll.html | 8 +++- .../editing/other/non-html-document.html | 24 ++++++++++ 8 files changed, 89 insertions(+), 31 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/wpt-import/editing/other/non-html-document.txt create mode 100644 Tests/LibWeb/Text/input/wpt-import/editing/other/non-html-document.html diff --git a/Libraries/LibWeb/DOM/Document.h b/Libraries/LibWeb/DOM/Document.h index dedae9725fa..cce5c4f496b 100644 --- a/Libraries/LibWeb/DOM/Document.h +++ b/Libraries/LibWeb/DOM/Document.h @@ -581,12 +581,12 @@ public: void set_previous_document_unload_timing(DocumentUnloadTimingInfo const& previous_document_unload_timing) { m_previous_document_unload_timing = previous_document_unload_timing; } // https://w3c.github.io/editing/docs/execCommand/ - bool exec_command(FlyString const& command, bool show_ui, String const& value); - bool query_command_enabled(FlyString const& command); - bool query_command_indeterm(FlyString const& command); - bool query_command_state(FlyString const& command); - bool query_command_supported(FlyString const& command); - String query_command_value(FlyString const& command); + WebIDL::ExceptionOr exec_command(FlyString const& command, bool show_ui, String const& value); + WebIDL::ExceptionOr query_command_enabled(FlyString const& command); + WebIDL::ExceptionOr query_command_indeterm(FlyString const& command); + WebIDL::ExceptionOr query_command_state(FlyString const& command); + WebIDL::ExceptionOr query_command_supported(FlyString const& command); + WebIDL::ExceptionOr query_command_value(FlyString const& command); // https://w3c.github.io/selection-api/#dfn-has-scheduled-selectionchange-event bool has_scheduled_selectionchange_event() const { return m_has_scheduled_selectionchange_event; } diff --git a/Libraries/LibWeb/Editing/Commands.cpp b/Libraries/LibWeb/Editing/Commands.cpp index d30ce2bf4d3..20b2dd46cbe 100644 --- a/Libraries/LibWeb/Editing/Commands.cpp +++ b/Libraries/LibWeb/Editing/Commands.cpp @@ -52,7 +52,7 @@ bool command_back_color_action(DOM::Document& document, String const& value) bool command_bold_action(DOM::Document& document, String const&) { // If queryCommandState("bold") returns true, set the selection's value to "normal". - if (document.query_command_state(CommandNames::bold)) { + if (MUST(document.query_command_state(CommandNames::bold))) { set_the_selections_value(document, CommandNames::bold, "normal"_string); } @@ -1864,7 +1864,7 @@ bool command_insert_unordered_list_state(DOM::Document const& document) bool command_italic_action(DOM::Document& document, String const&) { // If queryCommandState("italic") returns true, set the selection's value to "normal". - if (document.query_command_state(CommandNames::italic)) { + if (MUST(document.query_command_state(CommandNames::italic))) { set_the_selections_value(document, CommandNames::italic, "normal"_string); } @@ -2256,7 +2256,7 @@ bool command_select_all_action(DOM::Document& document, String const&) bool command_strikethrough_action(DOM::Document& document, String const&) { // If queryCommandState("strikethrough") returns true, set the selection's value to null. - if (document.query_command_state(CommandNames::strikethrough)) { + if (MUST(document.query_command_state(CommandNames::strikethrough))) { set_the_selections_value(document, CommandNames::strikethrough, {}); } @@ -2291,7 +2291,7 @@ bool command_style_with_css_state(DOM::Document const& document) bool command_subscript_action(DOM::Document& document, String const&) { // 1. Call queryCommandState("subscript"), and let state be the result. - auto state = document.query_command_state(CommandNames::subscript); + auto state = MUST(document.query_command_state(CommandNames::subscript)); // 2. Set the selection's value to null. set_the_selections_value(document, CommandNames::subscript, {}); @@ -2344,7 +2344,7 @@ bool command_subscript_indeterminate(DOM::Document const& document) bool command_superscript_action(DOM::Document& document, String const&) { // 1. Call queryCommandState("superscript"), and let state be the result. - auto state = document.query_command_state(CommandNames::superscript); + auto state = MUST(document.query_command_state(CommandNames::superscript)); // 2. Set the selection's value to null. set_the_selections_value(document, CommandNames::superscript, {}); @@ -2397,7 +2397,7 @@ bool command_superscript_indeterminate(DOM::Document const& document) bool command_underline_action(DOM::Document& document, String const&) { // If queryCommandState("underline") returns true, set the selection's value to null. - if (document.query_command_state(CommandNames::underline)) { + if (MUST(document.query_command_state(CommandNames::underline))) { set_the_selections_value(document, CommandNames::underline, {}); } diff --git a/Libraries/LibWeb/Editing/ExecCommand.cpp b/Libraries/LibWeb/Editing/ExecCommand.cpp index 388caf833a9..b4405dbeae6 100644 --- a/Libraries/LibWeb/Editing/ExecCommand.cpp +++ b/Libraries/LibWeb/Editing/ExecCommand.cpp @@ -14,15 +14,19 @@ namespace Web::DOM { // https://w3c.github.io/editing/docs/execCommand/#execcommand() -bool Document::exec_command(FlyString const& command, [[maybe_unused]] bool show_ui, String const& value) +WebIDL::ExceptionOr Document::exec_command(FlyString const& command, [[maybe_unused]] bool show_ui, String const& value) { + // AD-HOC: This is not directly mentioned in the spec, but all major browsers limit editing API calls to HTML documents + if (!is_html_document()) + return WebIDL::InvalidStateError::create(realm(), "execCommand is only supported on HTML documents"_string); + // 1. If only one argument was provided, let show UI be false. // 2. If only one or two arguments were provided, let value be the empty string. // NOTE: these steps are dealt by the default values for both show_ui and value // 3. If command is not supported or not enabled, return false. // NOTE: query_command_enabled() also checks if command is supported - if (!query_command_enabled(command)) + if (!MUST(query_command_enabled(command))) return false; // 4. If command is not in the Miscellaneous commands section: @@ -54,7 +58,7 @@ bool Document::exec_command(FlyString const& command, [[maybe_unused]] bool show // // We have to check again whether the command is enabled, because the beforeinput handler // might have done something annoying like getSelection().removeAllRanges(). - if (!query_command_enabled(command)) + 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 @@ -99,10 +103,14 @@ bool Document::exec_command(FlyString const& command, [[maybe_unused]] bool show } // https://w3c.github.io/editing/docs/execCommand/#querycommandenabled() -bool Document::query_command_enabled(FlyString const& command) +WebIDL::ExceptionOr Document::query_command_enabled(FlyString const& command) { + // AD-HOC: This is not directly mentioned in the spec, but all major browsers limit editing API calls to HTML documents + if (!is_html_document()) + return WebIDL::InvalidStateError::create(realm(), "queryCommandEnabled is only supported on HTML documents"_string); + // 2. Return true if command is both supported and enabled, false otherwise. - if (!query_command_supported(command)) + if (!MUST(query_command_supported(command))) return false; // https://w3c.github.io/editing/docs/execCommand/#enabled @@ -119,7 +127,7 @@ bool Document::query_command_enabled(FlyString const& command) // AD-HOC: selectAll requires a selection object to exist. if (command == Editing::CommandNames::selectAll) - return get_selection(); + return get_selection() != nullptr; // The other commands defined here are enabled if the active range is not null, auto active_range = Editing::active_range(*this); @@ -179,8 +187,12 @@ bool Document::query_command_enabled(FlyString const& command) } // https://w3c.github.io/editing/docs/execCommand/#querycommandindeterm() -bool Document::query_command_indeterm(FlyString const& command) +WebIDL::ExceptionOr Document::query_command_indeterm(FlyString const& command) { + // AD-HOC: This is not directly mentioned in the spec, but all major browsers limit editing API calls to HTML documents + if (!is_html_document()) + return WebIDL::InvalidStateError::create(realm(), "queryCommandIndeterm is only supported on HTML documents"_string); + // 1. If command is not supported or has no indeterminacy, return false. auto optional_command = Editing::find_command_definition(command); if (!optional_command.has_value()) @@ -251,8 +263,12 @@ bool Document::query_command_indeterm(FlyString const& command) } // https://w3c.github.io/editing/docs/execCommand/#querycommandstate() -bool Document::query_command_state(FlyString const& command) +WebIDL::ExceptionOr Document::query_command_state(FlyString const& command) { + // AD-HOC: This is not directly mentioned in the spec, but all major browsers limit editing API calls to HTML documents + if (!is_html_document()) + return WebIDL::InvalidStateError::create(realm(), "queryCommandState is only supported on HTML documents"_string); + // 1. If command is not supported or has no state, return false. auto optional_command = Editing::find_command_definition(command); if (!optional_command.has_value()) @@ -293,8 +309,12 @@ bool Document::query_command_state(FlyString const& command) } // https://w3c.github.io/editing/docs/execCommand/#querycommandsupported() -bool Document::query_command_supported(FlyString const& command) +WebIDL::ExceptionOr Document::query_command_supported(FlyString const& command) { + // AD-HOC: This is not directly mentioned in the spec, but all major browsers limit editing API calls to HTML documents + if (!is_html_document()) + return WebIDL::InvalidStateError::create(realm(), "queryCommandSupported is only supported on HTML documents"_string); + // When the queryCommandSupported(command) method on the Document interface is invoked, the // user agent must return true if command is supported and available within the current script // on the current site, and false otherwise. @@ -302,16 +322,20 @@ bool Document::query_command_supported(FlyString const& command) } // https://w3c.github.io/editing/docs/execCommand/#querycommandvalue() -String Document::query_command_value(FlyString const& command) +WebIDL::ExceptionOr Document::query_command_value(FlyString const& command) { + // AD-HOC: This is not directly mentioned in the spec, but all major browsers limit editing API calls to HTML documents + if (!is_html_document()) + return WebIDL::InvalidStateError::create(realm(), "queryCommandValue is only supported on HTML documents"_string); + // 1. If command is not supported or has no value, return the empty string. auto optional_command = Editing::find_command_definition(command); if (!optional_command.has_value()) - return {}; + return String {}; auto const& command_definition = optional_command.release_value(); auto value_override = command_value_override(command); if (!command_definition.value && !value_override.has_value()) - return {}; + return String {}; // 2. If command is "fontSize" and its value override is set, convert the value override to an // integer number of pixels and return the legacy font size for the result. diff --git a/Libraries/LibWeb/Editing/Internal/Algorithms.cpp b/Libraries/LibWeb/Editing/Internal/Algorithms.cpp index 7da93a98bb1..6199ef24944 100644 --- a/Libraries/LibWeb/Editing/Internal/Algorithms.cpp +++ b/Libraries/LibWeb/Editing/Internal/Algorithms.cpp @@ -3323,7 +3323,7 @@ Vector record_current_states_and_values(DOM::Document const& d // 6. For each command in the list "fontName", "foreColor", "hiliteColor", in order: add (command, command's value) // to overrides. for (auto const& command : { CommandNames::fontName, CommandNames::foreColor, CommandNames::hiliteColor }) - overrides.empend(command, node->document().query_command_value(command)); + overrides.empend(command, MUST(node->document().query_command_value(command))); // 7. Add ("fontSize", node's effective command value for "fontSize") to overrides. effective_value = effective_command_value(node, CommandNames::fontSize); @@ -3506,7 +3506,7 @@ void restore_states_and_values(DOM::Document& document, Vector for (auto override : overrides) { // 1. If override is a boolean, and queryCommandState(command) returns something different from override, // take the action for command, with value equal to the empty string. - if (override.value.has() && document.query_command_state(override.command) != override.value.get()) { + if (override.value.has() && MUST(document.query_command_state(override.command)) != override.value.get()) { take_the_action_for_command(document, override.command, {}); } @@ -3514,7 +3514,7 @@ void restore_states_and_values(DOM::Document& document, Vector // queryCommandValue(command) returns something not equivalent to override, take the action for command, // with value equal to override. else if (override.value.has() && !override.command.is_one_of(CommandNames::createLink, CommandNames::fontSize) - && document.query_command_value(override.command) != override.value.get()) { + && MUST(document.query_command_value(override.command)) != override.value.get()) { take_the_action_for_command(document, override.command, override.value.get()); } @@ -3761,7 +3761,7 @@ void set_the_selections_value(DOM::Document& document, FlyString const& command, } // 5. Otherwise, if command is "createLink" or it has a value specified, set the value override to new value. - else if (command == CommandNames::createLink || !document.query_command_value(CommandNames::createLink).is_empty()) { + else if (command == CommandNames::createLink || !MUST(document.query_command_value(CommandNames::createLink)).is_empty()) { document.set_command_value_override(command, new_value.value()); } diff --git a/Tests/LibWeb/Text/expected/Editing/execCommand-selectAll.txt b/Tests/LibWeb/Text/expected/Editing/execCommand-selectAll.txt index 43f9cabbfac..ecc6b0c14d6 100644 --- a/Tests/LibWeb/Text/expected/Editing/execCommand-selectAll.txt +++ b/Tests/LibWeb/Text/expected/Editing/execCommand-selectAll.txt @@ -2,6 +2,6 @@ No range. DIV 0 - DIV 0 BODY 0 - BODY 5 true -false +queryCommandEnabled threw exception of type: InvalidStateError false Did not crash! diff --git a/Tests/LibWeb/Text/expected/wpt-import/editing/other/non-html-document.txt b/Tests/LibWeb/Text/expected/wpt-import/editing/other/non-html-document.txt new file mode 100644 index 00000000000..94bfd991f15 --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/editing/other/non-html-document.txt @@ -0,0 +1,6 @@ +Harness status: OK + +Found 1 tests + +1 Pass +Pass editing APIs on an XML document should be disabled \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/Editing/execCommand-selectAll.html b/Tests/LibWeb/Text/input/Editing/execCommand-selectAll.html index 3f883a64931..f4744eb7e01 100644 --- a/Tests/LibWeb/Text/input/Editing/execCommand-selectAll.html +++ b/Tests/LibWeb/Text/input/Editing/execCommand-selectAll.html @@ -41,8 +41,12 @@ document.implementation.createHTMLDocument(), ]; for (const doc of documents) { - println(doc.queryCommandEnabled('selectAll')); - doc.execCommand('selectAll'); + try { + println(doc.queryCommandEnabled('selectAll')); + doc.execCommand('selectAll'); + } catch (e) { + println(`queryCommandEnabled threw exception of type: ${e.name}`); + } } println('Did not crash!'); }); diff --git a/Tests/LibWeb/Text/input/wpt-import/editing/other/non-html-document.html b/Tests/LibWeb/Text/input/wpt-import/editing/other/non-html-document.html new file mode 100644 index 00000000000..3e467f3d5d9 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/editing/other/non-html-document.html @@ -0,0 +1,24 @@ + + +Non-HTML document tests + + +