From 07e13e9868b933c5d51271c5e483724485da36fe Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 2 Aug 2020 16:05:59 +0200 Subject: [PATCH] LibWeb: Only allow editing of elements with contenteditable="true" We now respect the contenteditable HTML attribute and only let you edit content inside explicitly editable elements. --- Base/res/html/misc/welcome.html | 2 +- Libraries/LibWeb/DOM/AttributeNames.h | 91 +++++++++++++------------- Libraries/LibWeb/DOM/Document.h | 4 ++ Libraries/LibWeb/DOM/Element.cpp | 18 +++++ Libraries/LibWeb/DOM/Element.h | 2 + Libraries/LibWeb/DOM/Node.cpp | 5 ++ Libraries/LibWeb/DOM/Node.h | 2 + Libraries/LibWeb/Layout/LayoutText.cpp | 3 + Libraries/LibWeb/Page/EventHandler.cpp | 50 +++++++------- 9 files changed, 107 insertions(+), 70 deletions(-) diff --git a/Base/res/html/misc/welcome.html b/Base/res/html/misc/welcome.html index dc1f7371e0d..6bd40f935ce 100644 --- a/Base/res/html/misc/welcome.html +++ b/Base/res/html/misc/welcome.html @@ -1,5 +1,5 @@ - + Welcome! diff --git a/Libraries/LibWeb/DOM/AttributeNames.h b/Libraries/LibWeb/DOM/AttributeNames.h index 4f45cde8c6e..5a1b5fcdded 100644 --- a/Libraries/LibWeb/DOM/AttributeNames.h +++ b/Libraries/LibWeb/DOM/AttributeNames.h @@ -34,51 +34,52 @@ namespace AttributeNames { void initialize(); -#define ENUMERATE_HTML_ATTRIBUTES \ - __ENUMERATE_HTML_ATTRIBUTE(abbr) \ - __ENUMERATE_HTML_ATTRIBUTE(accept) \ - __ENUMERATE_HTML_ATTRIBUTE(action) \ - __ENUMERATE_HTML_ATTRIBUTE(align) \ - __ENUMERATE_HTML_ATTRIBUTE(allow) \ - __ENUMERATE_HTML_ATTRIBUTE(alt) \ - __ENUMERATE_HTML_ATTRIBUTE(async) \ - __ENUMERATE_HTML_ATTRIBUTE(bgcolor) \ - __ENUMERATE_HTML_ATTRIBUTE(class_) \ - __ENUMERATE_HTML_ATTRIBUTE(colspan) \ - __ENUMERATE_HTML_ATTRIBUTE(data) \ - __ENUMERATE_HTML_ATTRIBUTE(download) \ - __ENUMERATE_HTML_ATTRIBUTE(defer) \ - __ENUMERATE_HTML_ATTRIBUTE(dirname) \ - __ENUMERATE_HTML_ATTRIBUTE(headers) \ - __ENUMERATE_HTML_ATTRIBUTE(height) \ - __ENUMERATE_HTML_ATTRIBUTE(href) \ - __ENUMERATE_HTML_ATTRIBUTE(hreflang) \ - __ENUMERATE_HTML_ATTRIBUTE(id) \ - __ENUMERATE_HTML_ATTRIBUTE(imagesizes) \ - __ENUMERATE_HTML_ATTRIBUTE(imagesrcset) \ - __ENUMERATE_HTML_ATTRIBUTE(integrity) \ - __ENUMERATE_HTML_ATTRIBUTE(lang) \ - __ENUMERATE_HTML_ATTRIBUTE(max) \ - __ENUMERATE_HTML_ATTRIBUTE(media) \ - __ENUMERATE_HTML_ATTRIBUTE(method) \ - __ENUMERATE_HTML_ATTRIBUTE(min) \ - __ENUMERATE_HTML_ATTRIBUTE(name) \ - __ENUMERATE_HTML_ATTRIBUTE(pattern) \ - __ENUMERATE_HTML_ATTRIBUTE(ping) \ - __ENUMERATE_HTML_ATTRIBUTE(placeholder) \ - __ENUMERATE_HTML_ATTRIBUTE(rel) \ - __ENUMERATE_HTML_ATTRIBUTE(size) \ - __ENUMERATE_HTML_ATTRIBUTE(sizes) \ - __ENUMERATE_HTML_ATTRIBUTE(src) \ - __ENUMERATE_HTML_ATTRIBUTE(srcdoc) \ - __ENUMERATE_HTML_ATTRIBUTE(srcset) \ - __ENUMERATE_HTML_ATTRIBUTE(step) \ - __ENUMERATE_HTML_ATTRIBUTE(style) \ - __ENUMERATE_HTML_ATTRIBUTE(target) \ - __ENUMERATE_HTML_ATTRIBUTE(title) \ - __ENUMERATE_HTML_ATTRIBUTE(type) \ - __ENUMERATE_HTML_ATTRIBUTE(usemap) \ - __ENUMERATE_HTML_ATTRIBUTE(value) \ +#define ENUMERATE_HTML_ATTRIBUTES \ + __ENUMERATE_HTML_ATTRIBUTE(abbr) \ + __ENUMERATE_HTML_ATTRIBUTE(accept) \ + __ENUMERATE_HTML_ATTRIBUTE(action) \ + __ENUMERATE_HTML_ATTRIBUTE(align) \ + __ENUMERATE_HTML_ATTRIBUTE(allow) \ + __ENUMERATE_HTML_ATTRIBUTE(alt) \ + __ENUMERATE_HTML_ATTRIBUTE(async) \ + __ENUMERATE_HTML_ATTRIBUTE(bgcolor) \ + __ENUMERATE_HTML_ATTRIBUTE(class_) \ + __ENUMERATE_HTML_ATTRIBUTE(colspan) \ + __ENUMERATE_HTML_ATTRIBUTE(contenteditable) \ + __ENUMERATE_HTML_ATTRIBUTE(data) \ + __ENUMERATE_HTML_ATTRIBUTE(download) \ + __ENUMERATE_HTML_ATTRIBUTE(defer) \ + __ENUMERATE_HTML_ATTRIBUTE(dirname) \ + __ENUMERATE_HTML_ATTRIBUTE(headers) \ + __ENUMERATE_HTML_ATTRIBUTE(height) \ + __ENUMERATE_HTML_ATTRIBUTE(href) \ + __ENUMERATE_HTML_ATTRIBUTE(hreflang) \ + __ENUMERATE_HTML_ATTRIBUTE(id) \ + __ENUMERATE_HTML_ATTRIBUTE(imagesizes) \ + __ENUMERATE_HTML_ATTRIBUTE(imagesrcset) \ + __ENUMERATE_HTML_ATTRIBUTE(integrity) \ + __ENUMERATE_HTML_ATTRIBUTE(lang) \ + __ENUMERATE_HTML_ATTRIBUTE(max) \ + __ENUMERATE_HTML_ATTRIBUTE(media) \ + __ENUMERATE_HTML_ATTRIBUTE(method) \ + __ENUMERATE_HTML_ATTRIBUTE(min) \ + __ENUMERATE_HTML_ATTRIBUTE(name) \ + __ENUMERATE_HTML_ATTRIBUTE(pattern) \ + __ENUMERATE_HTML_ATTRIBUTE(ping) \ + __ENUMERATE_HTML_ATTRIBUTE(placeholder) \ + __ENUMERATE_HTML_ATTRIBUTE(rel) \ + __ENUMERATE_HTML_ATTRIBUTE(size) \ + __ENUMERATE_HTML_ATTRIBUTE(sizes) \ + __ENUMERATE_HTML_ATTRIBUTE(src) \ + __ENUMERATE_HTML_ATTRIBUTE(srcdoc) \ + __ENUMERATE_HTML_ATTRIBUTE(srcset) \ + __ENUMERATE_HTML_ATTRIBUTE(step) \ + __ENUMERATE_HTML_ATTRIBUTE(style) \ + __ENUMERATE_HTML_ATTRIBUTE(target) \ + __ENUMERATE_HTML_ATTRIBUTE(title) \ + __ENUMERATE_HTML_ATTRIBUTE(type) \ + __ENUMERATE_HTML_ATTRIBUTE(usemap) \ + __ENUMERATE_HTML_ATTRIBUTE(value) \ __ENUMERATE_HTML_ATTRIBUTE(width) #define __ENUMERATE_HTML_ATTRIBUTE(name) extern FlyString name; diff --git a/Libraries/LibWeb/DOM/Document.h b/Libraries/LibWeb/DOM/Document.h index 3f8bfe7cf17..6997e1b4934 100644 --- a/Libraries/LibWeb/DOM/Document.h +++ b/Libraries/LibWeb/DOM/Document.h @@ -157,6 +157,9 @@ public: const DocumentType* doctype() const; const String& compat_mode() const; + void set_editable(bool editable) { m_editable = editable; } + virtual bool is_editable() const final; + private: virtual RefPtr create_layout_node(const CSS::StyleProperties* parent_style) override; @@ -186,6 +189,7 @@ private: NonnullRefPtrVector m_scripts_to_execute_as_soon_as_possible; QuirksMode m_quirks_mode { QuirksMode::No }; + bool m_editable { false }; }; } diff --git a/Libraries/LibWeb/DOM/Element.cpp b/Libraries/LibWeb/DOM/Element.cpp index 78a2fc71206..6c8b08b47d4 100644 --- a/Libraries/LibWeb/DOM/Element.cpp +++ b/Libraries/LibWeb/DOM/Element.cpp @@ -290,4 +290,22 @@ String Element::inner_html() const return builder.to_string(); } +bool Element::is_editable() const +{ + auto contenteditable = attribute(HTML::AttributeNames::contenteditable); + // "true" and the empty string map to the "true" state. + if ((!contenteditable.is_null() && contenteditable.is_empty()) || contenteditable.equals_ignoring_case("true")) + return true; + // "false" maps to the "false" state. + if (contenteditable.equals_ignoring_case("false")) + return false; + // "inherit", an invalid value, and a missing value all map to the "inherit" state. + return parent() && parent()->is_editable(); +} + +bool Document::is_editable() const +{ + return m_editable; +} + } diff --git a/Libraries/LibWeb/DOM/Element.h b/Libraries/LibWeb/DOM/Element.h index 579bee58ba6..937db4382be 100644 --- a/Libraries/LibWeb/DOM/Element.h +++ b/Libraries/LibWeb/DOM/Element.h @@ -82,6 +82,8 @@ public: String inner_html() const; void set_inner_html(StringView); + virtual bool is_editable() const final; + protected: RefPtr create_layout_node(const CSS::StyleProperties* parent_style) override; diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index 744c0f27d2b..4370a540c68 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -217,4 +217,9 @@ void Node::set_document(Badge, Document& document) m_document = &document; } +bool Node::is_editable() const +{ + return parent() && parent()->is_editable(); +} + } diff --git a/Libraries/LibWeb/DOM/Node.h b/Libraries/LibWeb/DOM/Node.h index cc73a7ac67f..1bf2ab9d9b5 100644 --- a/Libraries/LibWeb/DOM/Node.h +++ b/Libraries/LibWeb/DOM/Node.h @@ -75,6 +75,8 @@ public: bool is_parent_node() const { return is_element() || is_document() || is_document_fragment(); } virtual bool is_svg_element() const { return false; } + virtual bool is_editable() const; + RefPtr append_child(NonnullRefPtr, bool notify = true); RefPtr insert_before(NonnullRefPtr node, RefPtr child, bool notify = true); diff --git a/Libraries/LibWeb/Layout/LayoutText.cpp b/Libraries/LibWeb/Layout/LayoutText.cpp index 55696b5531d..61839b4bb13 100644 --- a/Libraries/LibWeb/Layout/LayoutText.cpp +++ b/Libraries/LibWeb/Layout/LayoutText.cpp @@ -117,6 +117,9 @@ void LayoutText::paint_cursor_if_needed(PaintContext& context, const LineBoxFrag if (!(frame().cursor_position().offset() >= (unsigned)fragment.start() && frame().cursor_position().offset() < (unsigned)(fragment.start() + fragment.length()))) return; + if (!fragment.layout_node().node() || !fragment.layout_node().node()->is_editable()) + return; + auto fragment_rect = fragment.absolute_rect(); float cursor_x = fragment_rect.x() + specified_style().font().width(fragment.text().substring_view(0, frame().cursor_position().offset() - fragment.start())); diff --git a/Libraries/LibWeb/Page/EventHandler.cpp b/Libraries/LibWeb/Page/EventHandler.cpp index bf03ab46e3a..76e6c3badb0 100644 --- a/Libraries/LibWeb/Page/EventHandler.cpp +++ b/Libraries/LibWeb/Page/EventHandler.cpp @@ -231,31 +231,33 @@ void EventHandler::dump_selection(const char* event_name) const bool EventHandler::handle_keydown(KeyCode key, unsigned, u32 code_point) { - // FIXME: Support backspacing across DOM node boundaries. - if (key == KeyCode::Key_Backspace && m_frame.cursor_position().offset() > 0) { - auto& text_node = downcast(*m_frame.cursor_position().node()); - StringBuilder builder; - builder.append(text_node.data().substring_view(0, m_frame.cursor_position().offset() - 1)); - builder.append(text_node.data().substring_view(m_frame.cursor_position().offset(), text_node.data().length() - m_frame.cursor_position().offset())); - text_node.set_data(builder.to_string()); - m_frame.set_cursor_position({ *m_frame.cursor_position().node(), m_frame.cursor_position().offset() - 1 }); - // FIXME: This should definitely use incremental layout invalidation instead! - text_node.document().force_layout(); - return true; - } + if (m_frame.cursor_position().node() && m_frame.cursor_position().node()->is_editable()) { + // FIXME: Support backspacing across DOM node boundaries. + if (key == KeyCode::Key_Backspace && m_frame.cursor_position().offset() > 0) { + auto& text_node = downcast(*m_frame.cursor_position().node()); + StringBuilder builder; + builder.append(text_node.data().substring_view(0, m_frame.cursor_position().offset() - 1)); + builder.append(text_node.data().substring_view(m_frame.cursor_position().offset(), text_node.data().length() - m_frame.cursor_position().offset())); + text_node.set_data(builder.to_string()); + m_frame.set_cursor_position({ *m_frame.cursor_position().node(), m_frame.cursor_position().offset() - 1 }); + // FIXME: This should definitely use incremental layout invalidation instead! + text_node.document().force_layout(); + return true; + } - if (code_point && m_frame.cursor_position().is_valid() && is(*m_frame.cursor_position().node())) { - auto& text_node = downcast(*m_frame.cursor_position().node()); - StringBuilder builder; - builder.append(text_node.data().substring_view(0, m_frame.cursor_position().offset())); - builder.append_codepoint(code_point); - builder.append(text_node.data().substring_view(m_frame.cursor_position().offset(), text_node.data().length() - m_frame.cursor_position().offset())); - text_node.set_data(builder.to_string()); - // FIXME: This will advance the cursor incorrectly when inserting multiple whitespaces (DOM vs layout whitespace collapse difference.) - m_frame.set_cursor_position({ *m_frame.cursor_position().node(), m_frame.cursor_position().offset() + 1 }); - // FIXME: This should definitely use incremental layout invalidation instead! - text_node.document().force_layout(); - return true; + if (code_point && m_frame.cursor_position().is_valid() && is(*m_frame.cursor_position().node())) { + auto& text_node = downcast(*m_frame.cursor_position().node()); + StringBuilder builder; + builder.append(text_node.data().substring_view(0, m_frame.cursor_position().offset())); + builder.append_codepoint(code_point); + builder.append(text_node.data().substring_view(m_frame.cursor_position().offset(), text_node.data().length() - m_frame.cursor_position().offset())); + text_node.set_data(builder.to_string()); + // FIXME: This will advance the cursor incorrectly when inserting multiple whitespaces (DOM vs layout whitespace collapse difference.) + m_frame.set_cursor_position({ *m_frame.cursor_position().node(), m_frame.cursor_position().offset() + 1 }); + // FIXME: This should definitely use incremental layout invalidation instead! + text_node.document().force_layout(); + return true; + } } return false; }