commit bd3adf283792b42b85cd5fe1b0283d8c44f3b455 parent c32603b9b8039df752fef3ed0d96164f002e5562 Author: Masayuki Nakano <masayuki@d-toybox.com> Date: Fri, 12 Dec 2025 07:05:48 +0000 Bug 2003973 - Make `HTMLEditor` check whether selection is in a replaced or a void element stricter r=m_kato Currently, we do something special only for `<select>`, `<option>` and `<optgroup>`. However, we should treat replaced elements and void elements more consistently. This patch adds new `HTMLEditUtils` methods to check them easier and stricter and get the root node of them because DOM API can make invalid tree even in a void element. Then, we should handle insertions before the replaced or the void element if selection range is collapsed in it. However, deletion should not be handled for that if the range is completely in such element. Additionally, if range boundary is in such element, we should adjust the range not to include such element since it's not fully selected so that shouldn't be deleted nor replaced. These rules are basically compatible with Chrome. Finally, if such element is completely hidden, such elements should be deleted silently with deleting surrounding content if collapsed next to one. However, this does not work well with a `Backspace` key press even after applying this patch. Finally, some tests of `editing-around-select-element.tentative.html` start failing. The reason is, this patch makes the surrounding block boundary of `<select>` with adjusting the selection range and that makes `<select>` moved into another parent. At this time, the `<select>` gets `style="white-space: nowrap"`. I'm not sure where did this but not a critical failures. Therefore, this patch contains this regression. Differential Revision: https://phabricator.services.mozilla.com/D275170 Diffstat:
20 files changed, 605 insertions(+), 109 deletions(-)
diff --git a/editor/libeditor/AutoClonedRangeArray.cpp b/editor/libeditor/AutoClonedRangeArray.cpp @@ -35,6 +35,7 @@ namespace mozilla { using namespace dom; using EmptyCheckOption = HTMLEditUtils::EmptyCheckOption; +using ReplaceOrVoidElementOption = HTMLEditUtils::ReplaceOrVoidElementOption; /****************************************************************************** * mozilla::AutoClonedRangeArray @@ -97,25 +98,15 @@ bool AutoClonedRangeArray::IsEditableRange(const dom::AbstractRange& aRange, // first/last point of non-editable element. // See https://github.com/w3c/editing/issues/283#issuecomment-788654850 EditorRawDOMPoint atStart(aRange.StartRef()); - const bool isStartEditable = - atStart.IsInContentNode() && - EditorUtils::IsEditableContent(*atStart.ContainerAs<nsIContent>(), - EditorUtils::EditorType::HTML) && - !HTMLEditUtils::IsNonEditableReplacedContent( - *atStart.ContainerAs<nsIContent>()); - if (!isStartEditable) { + if (!atStart.IsInContentNode() || !HTMLEditUtils::IsSimplyEditableNode( + *atStart.ContainerAs<nsIContent>())) { return false; } if (aRange.GetStartContainer() != aRange.GetEndContainer()) { EditorRawDOMPoint atEnd(aRange.EndRef()); - const bool isEndEditable = - atEnd.IsInContentNode() && - EditorUtils::IsEditableContent(*atEnd.ContainerAs<nsIContent>(), - EditorUtils::EditorType::HTML) && - !HTMLEditUtils::IsNonEditableReplacedContent( - *atEnd.ContainerAs<nsIContent>()); - if (!isEndEditable) { + if (!atEnd.IsInContentNode() || !HTMLEditUtils::IsSimplyEditableNode( + *atEnd.ContainerAs<nsIContent>())) { return false; } @@ -166,6 +157,84 @@ void AutoClonedRangeArray::EnsureOnlyEditableRanges( mAnchorFocusRange = mRanges.IsEmpty() ? nullptr : mRanges.LastElement().get(); } +bool AutoClonedRangeArray::AdjustRangesNotInReplacedNorVoidElements( + RangeInReplacedOrVoidElement aRangeInReplacedOrVoidElement, + const dom::Element& aEditingHost) { + bool adjusted = false; + for (const size_t index : Reversed(IntegerRange(mRanges.Length()))) { + const OwningNonNull<nsRange>& range = mRanges[index]; + // If the range is in a replaced element or a void element, we should adjust + // the range boundaries outside of the element. + if (Element* const replacedOrVoidElementAtStart = + HTMLEditUtils::GetInclusiveAncestorReplacedOrVoidElement( + *range->StartRef().GetContainer()->AsContent(), + ReplaceOrVoidElementOption::LookForReplacedOrVoidElement)) { + adjusted = true; + if (MOZ_UNLIKELY(!replacedOrVoidElementAtStart->IsInclusiveDescendantOf( + &aEditingHost))) { + mRanges.RemoveElementAt(index); + continue; + } + nsIContent* const commonAncestorContent = + nsIContent::FromNode(range->GetClosestCommonInclusiveAncestor()); + if (commonAncestorContent && + commonAncestorContent->IsInclusiveDescendantOf( + replacedOrVoidElementAtStart)) { + // If the range is completely in a replaced element or a void element, + // let's treat that it's collapsed before the element or just delete the + // range. + if (aRangeInReplacedOrVoidElement == + RangeInReplacedOrVoidElement::Delete || + NS_WARN_IF(NS_FAILED(range->CollapseTo(RawRangeBoundary( + replacedOrVoidElementAtStart->GetParentNode(), + replacedOrVoidElementAtStart->GetPreviousSibling())))) || + MOZ_UNLIKELY( + !AutoClonedRangeArray::IsEditableRange(range, aEditingHost))) { + mRanges.RemoveElementAt(index); + continue; + } + adjusted = true; + } else { + // If the range does not end in the replaced element or the void + // element, let's treat that the range starts after the element. + if (NS_WARN_IF(NS_FAILED(range->SetStartAndEnd( + RawRangeBoundary(replacedOrVoidElementAtStart->GetParentNode(), + replacedOrVoidElementAtStart), + range->EndRef()))) || + MOZ_UNLIKELY( + !AutoClonedRangeArray::IsEditableRange(range, aEditingHost))) { + mRanges.RemoveElementAt(index); + continue; + } + } + } + if (!range->Collapsed() && + range->GetStartContainer() != range->GetEndContainer()) { + if (Element* const replacedOrVoidElementAtEnd = + HTMLEditUtils::GetInclusiveAncestorReplacedOrVoidElement( + *range->EndRef().GetContainer()->AsContent(), + ReplaceOrVoidElementOption::LookForReplacedOrVoidElement)) { + MOZ_ASSERT( + replacedOrVoidElementAtEnd->IsInclusiveDescendantOf(&aEditingHost)); + adjusted = true; + // If the range ends in a replaced element or a void element, let's + // treat that the range ends before the element. + if (NS_WARN_IF(NS_FAILED(range->SetStartAndEnd( + range->StartRef(), + RawRangeBoundary( + replacedOrVoidElementAtEnd->GetParentNode(), + replacedOrVoidElementAtEnd->GetPreviousSibling())))) || + MOZ_UNLIKELY( + !AutoClonedRangeArray::IsEditableRange(range, aEditingHost))) { + mRanges.RemoveElementAt(index); + continue; + } + } + } + } + return adjusted; +} + void AutoClonedRangeArray::EnsureRangesInTextNode(const Text& aTextNode) { auto GetOffsetInTextNode = [&aTextNode](const nsINode* aNode, uint32_t aOffset) -> uint32_t { diff --git a/editor/libeditor/AutoClonedRangeArray.h b/editor/libeditor/AutoClonedRangeArray.h @@ -77,6 +77,25 @@ class MOZ_STACK_CLASS AutoClonedRangeArray { */ void EnsureOnlyEditableRanges(const dom::Element& aEditingHost); + enum class RangeInReplacedOrVoidElement : bool { + // Each range in a replaced or a void element should be collapsed before the + // element. + Collapse, + // Each range in a replaced or a void element should be deleted. + Delete, + }; + + /** + * Adjust ranges if each boundary is in a replaced element or a void element. + * If the adjusted range is not at proper position to edit, this will remove + * the range. + * + * @return true if some ranges are modified. + */ + bool AdjustRangesNotInReplacedNorVoidElements( + RangeInReplacedOrVoidElement aRangeInReplacedOrVoidElement, + const dom::Element& aEditingHost); + /** * EnsureRangesInTextNode() is designed for TextEditor to guarantee that * all ranges are in its text node which is first child of the anonymous <div> diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp @@ -4799,7 +4799,17 @@ nsresult EditorBase::DeleteSelectionAsSubAction( Result<EditActionResult, nsresult> result = HandleDeleteSelection(aDirectionAndAmount, aStripWrappers); if (MOZ_UNLIKELY(result.isErr())) { - NS_WARNING("TextEditor::HandleDeleteSelection() failed"); + // If HTMLEditor::HandleDeleteSelection() returns "no editable range" + // error and the range is collapsed and the deletion is a preparation for + // inserting something, we wan't to keep handling the insertion without + // error. + if (result.inspectErr() == NS_ERROR_EDITOR_NO_DELETABLE_RANGE && + GetTopLevelEditSubAction() != EditSubAction::eDeleteSelectedContent) { + return NS_OK; + } + NS_WARNING(nsPrintfCString("%s::HandleDeleteSelection() failed", + IsTextEditor() ? "TextEditor" : "HTMLEditor") + .get()); return result.unwrapErr(); } if (result.inspect().Canceled()) { diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp @@ -671,8 +671,8 @@ nsresult HTMLEditor::OnEndHandlingTopLevelEditSubActionInternal() { } Result<EditActionResult, nsresult> HTMLEditor::CanHandleHTMLEditSubAction( - CheckSelectionInReplacedElement aCheckSelectionInReplacedElement - /* = CheckSelectionInReplacedElement::Yes */) const { + CheckSelectionInReplacedElement + aCheckSelectionInReplacedElement /* = ::Yes */) const { MOZ_ASSERT(IsEditActionDataAvailable()); if (NS_WARN_IF(Destroyed())) { @@ -699,18 +699,26 @@ Result<EditActionResult, nsresult> HTMLEditor::CanHandleHTMLEditSubAction( return Err(NS_ERROR_FAILURE); } + using ReplaceOrVoidElementOption = HTMLEditUtils::ReplaceOrVoidElementOption; + if (selStartNode == selEndNode) { if (aCheckSelectionInReplacedElement == CheckSelectionInReplacedElement::Yes && - HTMLEditUtils::IsNonEditableReplacedContent( - *selStartNode->AsContent())) { + HTMLEditUtils::GetInclusiveAncestorReplacedOrVoidElement( + *selStartNode->AsContent(), + ReplaceOrVoidElementOption::LookForOnlyNonVoidReplacedElement)) { return EditActionResult::CanceledResult(); } return EditActionResult::IgnoredResult(); } - if (HTMLEditUtils::IsNonEditableReplacedContent(*selStartNode->AsContent()) || - HTMLEditUtils::IsNonEditableReplacedContent(*selEndNode->AsContent())) { + if (aCheckSelectionInReplacedElement != CheckSelectionInReplacedElement::No && + (HTMLEditUtils::GetInclusiveAncestorReplacedOrVoidElement( + *selStartNode->AsContent(), + ReplaceOrVoidElementOption::LookForOnlyNonVoidReplacedElement) || + HTMLEditUtils::GetInclusiveAncestorReplacedOrVoidElement( + *selEndNode->AsContent(), + ReplaceOrVoidElementOption::LookForOnlyNonVoidReplacedElement))) { return EditActionResult::CanceledResult(); } @@ -972,7 +980,8 @@ Result<EditActionResult, nsresult> HTMLEditor::HandleInsertText( ToString(aPurpose).c_str())); { - Result<EditActionResult, nsresult> result = CanHandleHTMLEditSubAction(); + Result<EditActionResult, nsresult> result = + CanHandleHTMLEditSubAction(CheckSelectionInReplacedElement::No); if (MOZ_UNLIKELY(result.isErr())) { NS_WARNING("HTMLEditor::CanHandleHTMLEditSubAction() failed"); return result; @@ -1072,17 +1081,12 @@ Result<EditActionResult, nsresult> HTMLEditor::HandleInsertText( // If the point is not in an element which can contain text nodes, climb up // the DOM tree. - if (!pointToInsert.IsInTextNode()) { - while (!HTMLEditUtils::CanNodeContain(*pointToInsert.GetContainer(), - *nsGkAtoms::textTagName)) { - if (NS_WARN_IF(pointToInsert.GetContainer() == editingHost) || - NS_WARN_IF(!pointToInsert.GetContainerParentAs<nsIContent>())) { - NS_WARNING("Selection start point couldn't have text nodes"); - return Err(NS_ERROR_FAILURE); - } - pointToInsert.Set(pointToInsert.ContainerAs<nsIContent>()); - } + pointToInsert = HTMLEditUtils::GetPossiblePointToInsert( + pointToInsert, *nsGkAtoms::textTagName, *editingHost); + if (NS_WARN_IF(!pointToInsert.IsSet())) { + return Err(NS_ERROR_FAILURE); } + MOZ_ASSERT(pointToInsert.IsInContentNode()); if (InsertingTextForComposition(aPurpose)) { if (aInsertionString.IsEmpty()) { diff --git a/editor/libeditor/HTMLEditUtils.cpp b/editor/libeditor/HTMLEditUtils.cpp @@ -41,7 +41,8 @@ #include "nsError.h" // for NS_SUCCEEDED #include "nsGkAtoms.h" // for nsGkAtoms, nsGkAtoms::a, etc. #include "nsHTMLTags.h" -#include "nsIContentInlines.h" // for nsIContent::IsInDesignMode(), etc. +#include "nsIContentInlines.h" // for nsIContent::IsInDesignMode(), etc. +#include "nsIObjectLoadingContent.h" #include "nsLiteralString.h" // for NS_LITERAL_STRING #include "nsNameSpaceManager.h" // for kNameSpaceID_None #include "nsPrintfCString.h" // nsPringfCString @@ -652,6 +653,42 @@ bool HTMLEditUtils::IsMailCiteElement(const Element& aElement) { return false; } +bool HTMLEditUtils::IsReplacedElement(const Element& aElement) { + if (!aElement.IsHTMLElement()) { + // FIXME: Well known SVG, MathML elements should be tested here. + return false; + } + if (aElement.IsHTMLElement(nsGkAtoms::input)) { + return !aElement.AttrValueIs(kNameSpaceID_None, nsGkAtoms::type, + nsGkAtoms::hidden, eIgnoreCase); + } + if (HTMLEditUtils::IsFormWidgetElement(aElement)) { + return true; + } + // <object> is a special element, it shows its subtree when it does not load + // its content. + if (aElement.IsHTMLElement(nsGkAtoms::object)) { + const nsCOMPtr<nsIObjectLoadingContent> objectLoadingContent = + do_QueryInterface(const_cast<Element*>(&aElement)); + uint32_t displayedType = nsIObjectLoadingContent::TYPE_FALLBACK; + if (MOZ_LIKELY(objectLoadingContent)) { + objectLoadingContent->GetDisplayedType(&displayedType); + } + return displayedType != nsIObjectLoadingContent::TYPE_FALLBACK; + } + return aElement.IsAnyOfHTMLElements( + nsGkAtoms::audio, + // In strictly speaking, <br> is not a replaced element, but treating it + // as a replaced element makes HTMLEditor and its peers simpler. + nsGkAtoms::br, nsGkAtoms::canvas, nsGkAtoms::embed, nsGkAtoms::iframe, + nsGkAtoms::img, + // <optgroup> and <option> are not replaced element actually but they + // are treated as so for the compatibility with Chrome. + // XXX I wonder if we can treat them as so only when they are in + // <select>. + nsGkAtoms::optgroup, nsGkAtoms::option, nsGkAtoms::video); +} + bool HTMLEditUtils::IsFormWidgetElement(const nsIContent& aContent) { return aContent.IsAnyOfHTMLElements(nsGkAtoms::textarea, nsGkAtoms::select, nsGkAtoms::button, nsGkAtoms::output, @@ -1381,13 +1418,17 @@ bool HTMLEditUtils::IsEmptyNode(nsPresContext* aPresContext, if ( // If it's not a container such as an <hr> or <br>, etc, it should be // treated as not empty. + // XXX I think <input type="hidden"> should not be treated as a special + // element since it's invisible. Treating invisible elements as special + // ones causes changing the behavior with the invisible thing so that the + // users may report the different behavior as a bug. !IsContainerNode(*aNode.AsContent()) || // If it's a named anchor, we shouldn't treat it as empty because it // has special meaning even if invisible. IsNamedAnchorElement(*aNode.AsContent()) || - // Form widgets should be treated as not empty because they have special - // meaning even if invisible. - IsFormWidgetElement(*aNode.AsContent())) { + // Replaced elements should be treated as not empty because they have + // visible content. + IsReplacedElement(*aNode.AsElement())) { return false; } diff --git a/editor/libeditor/HTMLEditUtils.h b/editor/libeditor/HTMLEditUtils.h @@ -108,27 +108,56 @@ class HTMLEditUtils final { */ static bool IsNeverElementContentsEditableByUser(const nsIContent& aContent) { return aContent.IsElement() && + // XXX I think we should not treat <button> contents as editable + !aContent.IsHTMLElement(nsGkAtoms::button) && (!HTMLEditUtils::IsContainerNode(aContent) || - aContent.IsAnyOfHTMLElements( - nsGkAtoms::applet, nsGkAtoms::colgroup, nsGkAtoms::frameset, - nsGkAtoms::head, nsGkAtoms::html, nsGkAtoms::iframe, - nsGkAtoms::meter, nsGkAtoms::progress, nsGkAtoms::select, - nsGkAtoms::textarea)); + HTMLEditUtils::IsReplacedElement(*aContent.AsElement()) || + aContent.IsAnyOfHTMLElements(nsGkAtoms::applet, nsGkAtoms::colgroup, + nsGkAtoms::frameset, nsGkAtoms::head, + nsGkAtoms::html)); } + enum class ReplaceOrVoidElementOption { + LookForOnlyVoidElement, + LookForOnlyReplaceElement, + LookForOnlyNonVoidReplacedElement, + LookForReplacedOrVoidElement, + }; + /** - * IsNonEditableReplacedContent() returns true when aContent is an inclusive - * descendant of a replaced element whose content shouldn't be editable by - * user's operation. - */ - static bool IsNonEditableReplacedContent(const nsIContent& aContent) { - for (Element* element : aContent.InclusiveAncestorsOfType<Element>()) { - if (element->IsAnyOfHTMLElements(nsGkAtoms::select, nsGkAtoms::option, - nsGkAtoms::optgroup)) { - return true; - } - } - return false; + * Return an inclusive ancestor replaced element or void element of aContent. + * I.e., if this returns non-nullptr, aContent is in a replaced element or a + * void element. + */ + [[nodiscard]] static Element* GetInclusiveAncestorReplacedOrVoidElement( + const nsIContent& aContent, ReplaceOrVoidElementOption aOption) { + const bool lookForAnyReplaceElement = + aOption == ReplaceOrVoidElementOption::LookForOnlyReplaceElement || + aOption == ReplaceOrVoidElementOption::LookForReplacedOrVoidElement; + const bool lookForNonVoidReplacedElement = + aOption == + ReplaceOrVoidElementOption::LookForOnlyNonVoidReplacedElement; + const bool lookForVoidElement = + aOption == ReplaceOrVoidElementOption::LookForOnlyVoidElement || + aOption == ReplaceOrVoidElementOption::LookForReplacedOrVoidElement; + Element* lastReplacedOrVoidElement = nullptr; + for (Element* const element : + aContent.InclusiveAncestorsOfType<Element>()) { + // XXX I think we should not treat <button> contents as editable + if (lookForAnyReplaceElement && + !element->IsHTMLElement(nsGkAtoms::button) && + HTMLEditUtils::IsReplacedElement(*element)) { + lastReplacedOrVoidElement = element; + } else if (lookForNonVoidReplacedElement && + !element->IsHTMLElement(nsGkAtoms::button) && + HTMLEditUtils::IsNonVoidReplacedElement(*element)) { + lastReplacedOrVoidElement = element; + } else if (lookForVoidElement && + !HTMLEditUtils::IsContainerNode(*element)) { + lastReplacedOrVoidElement = element; + } + } + return lastReplacedOrVoidElement; } /* @@ -423,6 +452,19 @@ class HTMLEditUtils final { [[nodiscard]] static bool IsMailCiteElement(const Element& aElement); /** + * Return true if aElement is a replaced element. + */ + [[nodiscard]] static bool IsReplacedElement(const Element& aElement); + + /** + * Return true if aElement is a non-void replaced element such as <iframe>, + * <embed>, <audio>, <video>, <select>, etc. + */ + [[nodiscard]] static bool IsNonVoidReplacedElement(const Element& aElement) { + return IsReplacedElement(aElement) && IsContainerNode(aElement); + } + + /** * Return true if aElement is a form widget, i.e., a replaced element for the * <form>. */ @@ -489,6 +531,64 @@ class HTMLEditUtils final { } /** + * Return a point where can insert a node whose name is aInsertNodeName. + * Note that if the container of aPointToInsert is not an element, this check + * whether aInsertNodeName can be inserted into the element. Therefore, the + * caller may need to split the container when actually inserting a node. + */ + [[nodiscard]] static EditorDOMPoint GetPossiblePointToInsert( + const EditorDOMPoint& aPointToInsert, const nsAtom& aInsertNodeName, + const Element& aEditingHost) { + if (MOZ_UNLIKELY(!aPointToInsert.IsInContentNode())) { + return EditorDOMPoint(); + } + EditorDOMPoint pointToInsert(aPointToInsert); + // We shouldn't modify the subtree in a replaced element so that we need to + // test whether aInsertNodeName is inserted with inclusive ancestors + // starting from the most distant replaced element ancestor. + if (Element* const replacedOrVoidElement = + HTMLEditUtils::GetInclusiveAncestorReplacedOrVoidElement( + *aPointToInsert.GetContainer()->AsContent(), + ReplaceOrVoidElementOption::LookForReplacedOrVoidElement)) { + if (MOZ_UNLIKELY(replacedOrVoidElement == &aEditingHost) || + MOZ_UNLIKELY( + !replacedOrVoidElement->IsInclusiveDescendantOf(&aEditingHost))) { + return EditorDOMPoint(); + } + pointToInsert.Set(replacedOrVoidElement); + } + if ((pointToInsert.IsInTextNode() && + &aInsertNodeName == nsGkAtoms::textTagName) || + HTMLEditUtils::CanNodeContain(*pointToInsert.GetContainer(), + aInsertNodeName)) { + return pointToInsert; + } + if (pointToInsert.IsInTextNode()) { + Element* const parentElement = + pointToInsert.GetContainerParentAs<Element>(); + if (NS_WARN_IF(!parentElement)) { + return EditorDOMPoint(); + } + if (HTMLEditUtils::CanNodeContain(*parentElement, aInsertNodeName)) { + // Okay, the insertion point should be fine even though the caller needs + // to split the `Text`. + return pointToInsert; + } + } + nsIContent* lastContent = pointToInsert.GetContainer()->AsContent(); + for (Element* const element : lastContent->AncestorsOfType<Element>()) { + if (HTMLEditUtils::CanNodeContain(*element, aInsertNodeName)) { + return EditorDOMPoint(lastContent); + } + if (MOZ_UNLIKELY(element == &aEditingHost)) { + return EditorDOMPoint(); + } + lastContent = element; + } + return pointToInsert; + } + + /** * CanElementContainParagraph() returns true if aElement can have a <p> * element as its child or its descendant. */ @@ -573,7 +673,9 @@ class HTMLEditUtils final { nsGkAtoms::tbody, nsGkAtoms::tfoot, nsGkAtoms::thead, nsGkAtoms::tr) && !HTMLEditUtils::IsNeverElementContentsEditableByUser(aContent) && - !HTMLEditUtils::IsNonEditableReplacedContent(aContent); + !HTMLEditUtils::GetInclusiveAncestorReplacedOrVoidElement( + aContent, + ReplaceOrVoidElementOption::LookForReplacedOrVoidElement); } return aContent.IsText() && aContent.Length() > 0; } diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h @@ -1131,7 +1131,7 @@ class HTMLEditor final : public EditorBase, * XXX I think that `IsSelectionEditable()` is better name, but it's already * in `EditorBase`... */ - enum class CheckSelectionInReplacedElement { Yes, OnlyWhenNotInSameNode }; + enum class CheckSelectionInReplacedElement { No, Yes, OnlyWhenNotInSameNode }; Result<EditActionResult, nsresult> CanHandleHTMLEditSubAction( CheckSelectionInReplacedElement aCheckSelectionInReplacedElement = CheckSelectionInReplacedElement::Yes) const; diff --git a/editor/libeditor/HTMLEditorDeleteHandler.cpp b/editor/libeditor/HTMLEditorDeleteHandler.cpp @@ -441,6 +441,15 @@ nsresult HTMLEditor::ComputeTargetRanges( "There is no range which we can delete entire of or around the caret"); return NS_ERROR_EDITOR_NO_EDITABLE_RANGE; } + // Delete each range if completely in a replaced element or a void element + // because collapsing the range outside may cause the surrounding content + // which is outside the selection range will be deleted. + if (aRangesToDelete.AdjustRangesNotInReplacedNorVoidElements( + AutoClonedRangeArray::RangeInReplacedOrVoidElement::Delete, + *editingHost) && + !aRangesToDelete.Ranges().Length()) { + return NS_ERROR_EDITOR_NO_EDITABLE_RANGE; + } AutoDeleteRangesHandler deleteHandler; // Should we delete target ranges which cannot delete actually? nsresult rv = deleteHandler.ComputeRangesToDelete( @@ -503,6 +512,39 @@ Result<EditActionResult, nsresult> HTMLEditor::HandleDeleteSelection( "caret"); return Err(NS_ERROR_EDITOR_NO_EDITABLE_RANGE); } + // Delete each range if completely in a replaced element or a void element + // because collapsing the range outside may cause the surrounding content + // which is outside the selection range will be deleted. + if (rangesToDelete.AdjustRangesNotInReplacedNorVoidElements( + AutoClonedRangeArray::RangeInReplacedOrVoidElement::Delete, + *editingHost) && + rangesToDelete.Ranges().IsEmpty()) { + // Collapse Selection to the first editable range to avoid the toplevel edit + // subaction handler to be confused at non-selection ranges. + if (GetTopLevelEditSubAction() != EditSubAction::eDeleteSelectedContent) { + AutoClonedSelectionRangeArray editableSelectionRanges(SelectionRef()); + editableSelectionRanges.EnsureOnlyEditableRanges(*editingHost); + if (!editableSelectionRanges.GetAncestorLimiter()) { + editableSelectionRanges.SetAncestorLimiter( + FindSelectionRoot(*editingHost)); + } + editableSelectionRanges.AdjustRangesNotInReplacedNorVoidElements( + AutoClonedRangeArray::RangeInReplacedOrVoidElement::Collapse, + *editingHost); + if (NS_WARN_IF(editableSelectionRanges.Ranges().IsEmpty())) { + return Err(NS_ERROR_FAILURE); + } + nsresult rv = editableSelectionRanges.Collapse( + editableSelectionRanges.GetFirstRangeStartPoint<EditorRawDOMPoint>()); + if (NS_WARN_IF(Destroyed())) { + return Err(NS_ERROR_EDITOR_DESTROYED); + } + if (NS_FAILED(rv)) { + return Err(rv); + } + } + return Err(NS_ERROR_EDITOR_NO_DELETABLE_RANGE); + } AutoDeleteRangesHandler deleteHandler; Result<EditActionResult, nsresult> result = deleteHandler.Run( *this, aDirectionAndAmount, aStripWrappers, rangesToDelete, *editingHost); diff --git a/editor/libeditor/HTMLEditorInsertLineBreakHandler.cpp b/editor/libeditor/HTMLEditorInsertLineBreakHandler.cpp @@ -58,7 +58,8 @@ nsresult HTMLEditor::InsertLineBreakAsSubAction() { } { - Result<EditActionResult, nsresult> result = CanHandleHTMLEditSubAction(); + Result<EditActionResult, nsresult> result = + CanHandleHTMLEditSubAction(CheckSelectionInReplacedElement::No); if (MOZ_UNLIKELY(result.isErr())) { NS_WARNING("HTMLEditor::CanHandleHTMLEditSubAction() failed"); return result.unwrapErr(); @@ -145,12 +146,23 @@ nsresult HTMLEditor::AutoInsertLineBreakHandler::Run() { } nsresult HTMLEditor::AutoInsertLineBreakHandler::HandleInsertBRElement() { - const auto atStartOfSelection = - mHTMLEditor.GetFirstSelectionStartPoint<EditorDOMPoint>(); - MOZ_ASSERT(atStartOfSelection.IsInContentNode()); + const EditorDOMPoint pointToInsert = [&]() { + const auto atStartOfSelection = + mHTMLEditor.GetFirstSelectionStartPoint<EditorDOMPoint>(); + MOZ_ASSERT(atStartOfSelection.IsInContentNode()); + return HTMLEditUtils::GetPossiblePointToInsert( + atStartOfSelection, *nsGkAtoms::br, mEditingHost); + }(); + if (NS_WARN_IF(!pointToInsert.IsSet())) { + return Err(NS_ERROR_FAILURE); + } + MOZ_ASSERT(pointToInsert.IsInContentNode()); + + // XXX Should we check the preferred line break again? + Result<CreateLineBreakResult, nsresult> insertLineBreakResultOrError = mHTMLEditor.InsertLineBreak(WithTransaction::Yes, - LineBreakType::BRElement, atStartOfSelection, + LineBreakType::BRElement, pointToInsert, nsIEditor::eNext); if (MOZ_UNLIKELY(insertLineBreakResultOrError.isErr())) { NS_WARNING( @@ -326,14 +338,16 @@ HTMLEditor::AutoInsertLineBreakHandler::InsertLinefeed( // The node may not be able to have a text node so that we need to check it // here. - if (!pointToInsert.IsInTextNode() && - !HTMLEditUtils::CanNodeContain(*pointToInsert.ContainerAs<nsIContent>(), - *nsGkAtoms::textTagName)) { - NS_WARNING( - "AutoInsertLineBreakHandler::InsertLinefeed() couldn't insert a " - "linefeed because the insertion position couldn't have text nodes"); - return Err(NS_ERROR_EDITOR_NO_EDITABLE_RANGE); + pointToInsert = HTMLEditUtils::GetPossiblePointToInsert( + pointToInsert, *nsGkAtoms::textTagName, aEditingHost); + if (NS_WARN_IF(!pointToInsert.IsSet())) { + return Err(NS_ERROR_FAILURE); } + MOZ_ASSERT(pointToInsert.IsInContentNode()); + + // FIXME: If the computed point does not preformat linefeed, we should switch + // back to inserting a <br>. However, I think it should be handled before + // calling this. AutoRestore<bool> disableListener( aHTMLEditor.EditSubActionDataRef().mAdjustChangedRangeFromListener); diff --git a/editor/libeditor/HTMLEditorInsertParagraphHandler.cpp b/editor/libeditor/HTMLEditorInsertParagraphHandler.cpp @@ -60,8 +60,8 @@ HTMLEditor::InsertParagraphSeparatorAsSubAction(const Element& aEditingHost) { } { - Result<EditActionResult, nsresult> result = CanHandleHTMLEditSubAction( - CheckSelectionInReplacedElement::OnlyWhenNotInSameNode); + Result<EditActionResult, nsresult> result = + CanHandleHTMLEditSubAction(CheckSelectionInReplacedElement::No); if (MOZ_UNLIKELY(result.isErr())) { NS_WARNING("HTMLEditor::CanHandleHTMLEditSubAction() failed"); return result; @@ -150,20 +150,15 @@ HTMLEditor::AutoInsertParagraphHandler::Run() { if (NS_WARN_IF(!pointToInsert.IsInContentNode())) { return Err(NS_ERROR_FAILURE); } - while (true) { - Element* element = pointToInsert.GetContainerOrContainerParentElement(); - if (MOZ_UNLIKELY(!element)) { - return Err(NS_ERROR_FAILURE); - } - // If the element can have a <br> element (it means that the element or its - // container must be able to have <div> or <p> too), we can handle - // insertParagraph at the point. - if (HTMLEditUtils::CanNodeContain(*element, *nsGkAtoms::br)) { - break; - } - // Otherwise, try to insert paragraph at the parent. - pointToInsert = pointToInsert.ParentPoint(); + // If the element can have a <br> element (it means that the element or its + // container must be able to have <div> or <p> too), we can handle + // insertParagraph at the point. + pointToInsert = HTMLEditUtils::GetPossiblePointToInsert( + pointToInsert, *nsGkAtoms::br, mEditingHost); + if (NS_WARN_IF(!pointToInsert.IsSet())) { + return Err(NS_ERROR_FAILURE); } + MOZ_ASSERT(pointToInsert.IsInContentNode()); if (mHTMLEditor.IsMailEditor()) { if (const RefPtr<Element> mailCiteElement = diff --git a/editor/libeditor/WhiteSpaceVisibilityKeeper.cpp b/editor/libeditor/WhiteSpaceVisibilityKeeper.cpp @@ -1326,7 +1326,8 @@ WhiteSpaceVisibilityKeeper::NormalizeWhiteSpacesToSplitAt( if (auto* const element = Element::FromNode(previousContent)) { if (HTMLEditUtils::IsBlockElement( *element, BlockInlineCheck::UseComputedDisplayStyle) || - HTMLEditUtils::IsNonEditableReplacedContent(*element)) { + !HTMLEditUtils::IsContainerNode(*element) || + HTMLEditUtils::IsReplacedElement(*element)) { break; } // Ignore invisible inline elements @@ -1365,7 +1366,8 @@ WhiteSpaceVisibilityKeeper::NormalizeWhiteSpacesToSplitAt( if (auto* const element = Element::FromNode(nextContent)) { if (HTMLEditUtils::IsBlockElement( *element, BlockInlineCheck::UseComputedDisplayStyle) || - HTMLEditUtils::IsNonEditableReplacedContent(*element)) { + !HTMLEditUtils::IsContainerNode(*element) || + HTMLEditUtils::IsReplacedElement(*element)) { break; } // Ignore invisible inline elements diff --git a/editor/libeditor/tests/browserscope/lib/richtext2/currentStatus.js b/editor/libeditor/tests/browserscope/lib/richtext2/currentStatus.js @@ -727,9 +727,6 @@ const knownFailures = { "D-Proposed-OL-LI-1_SO-dM": true, "D-Proposed-OL-LI-1_SO-body": true, "D-Proposed-OL-LI-1_SO-div": true, - "D-Proposed-HR.BR-1_SM-dM": true, - "D-Proposed-HR.BR-1_SM-body": true, - "D-Proposed-HR.BR-1_SM-div": true, "D-Proposed-TR2rs:2-1_SO1-dM": true, "D-Proposed-TR2rs:2-1_SO1-body": true, "D-Proposed-TR2rs:2-1_SO1-div": true, diff --git a/testing/web-platform/meta/editing/other/editing-around-select-element.tentative.html.ini b/testing/web-platform/meta/editing/other/editing-around-select-element.tentative.html.ini @@ -1,7 +1,24 @@ [editing-around-select-element.tentative.html?delete] + [execCommand(delete, false, "") in <div contenteditable><p>ab[c</p><select><option>d\]ef</option></select></div>: shouldn't modify in <option>] + expected: FAIL + + [execCommand(delete, false, "") in <div contenteditable><p>ab[c</p><select multiple><option>d\]ef</option></select></div>: shouldn't modify in <option>] + expected: FAIL + [editing-around-select-element.tentative.html?forwardDelete] expected: if (os == "win") and debug and (processor == "x86_64"): [OK, CRASH] + [execCommand(forwardDelete, false, "") in <div contenteditable><p>ab[c</p><select><option>d\]ef</option></select></div>: shouldn't modify in <option>] + expected: FAIL + + [execCommand(forwardDelete, false, "") in <div contenteditable><p>ab[c</p><select multiple><option>d\]ef</option></select></div>: shouldn't modify in <option>] + expected: FAIL + [editing-around-select-element.tentative.html?insertText] + [execCommand(insertText, false, "XYZ") in <div contenteditable><p>ab[c</p><select><option>d\]ef</option></select></div>: shouldn't modify in <option>] + expected: FAIL + + [execCommand(insertText, false, "XYZ") in <div contenteditable><p>ab[c</p><select multiple><option>d\]ef</option></select></div>: shouldn't modify in <option>] + expected: FAIL diff --git a/testing/web-platform/meta/editing/other/insertparagraph-in-non-splittable-element.html.ini b/testing/web-platform/meta/editing/other/insertparagraph-in-non-splittable-element.html.ini @@ -1,9 +0,0 @@ -[insertparagraph-in-non-splittable-element.html] - [insertParagraph in iframe of <div><iframe srcdoc="abc"></iframe></div>] - expected: FAIL - - [insertParagraph in optgroup of <div><select><optgroup><option>abc</option></optgroup></select></div>] - expected: FAIL - - [insertParagraph in select of <div><select><option>abc</option></select></div>] - expected: FAIL diff --git a/testing/web-platform/meta/editing/run/delete.html.ini b/testing/web-platform/meta/editing/run/delete.html.ini @@ -663,3 +663,9 @@ [[["delete",""\]\] "<div style=display:grid><span>abc</span><span style=display:contents><span>[\]def</span></span></div>" compare innerHTML] expected: FAIL + + [[["delete",""\]\] "abc<audio>def</audio>[\]ghi" compare innerHTML] + expected: FAIL + + [[["delete",""\]\] "abc<input type=hidden>[\]def" compare innerHTML] + expected: FAIL diff --git a/testing/web-platform/tests/editing/data/delete.js b/testing/web-platform/tests/editing/data/delete.js @@ -3444,4 +3444,60 @@ var browserTests = [ "<div style=\"display:grid\"><span style=\"display:contents\"><span>abcdef</span></span></div>", [true], {}], + +// <object> content should be deleted if it has only fallback content. +["<object>abc</object>[]def", + [["delete",""]], + "<object>ab</object>def", + [true], + {}], +["abc<object data=\"/images/green-1x1.png\">def</object>[]ghi", + [["delete",""]], + "abcghi", + [true], + {}], +["abc<object data=\"about:blank\">def</object>[]ghi", + [["delete",""]], + "abcghi", + [true], + {}], +// The following elements should be rendered with replaced content so that +// the element itself should be removed. +["abc<iframe>def</iframe>[]ghi", + [["delete",""]], + "abcghi", + [true], + {}], +["abc<meter>def</meter>[]ghi", + [["delete",""]], + "abcghi", + [true], + {}], +["abc<progress>def</progress>[]ghi", + [["delete",""]], + "abcghi", + [true], + {}], +["abc<audio controls>def</audio>[]ghi", + [["delete",""]], + "abcghi", + [true], + {}], +["abc<video>def</video>[]ghi", + [["delete",""]], + "abcghi", + [true], + {}], +// <audio> and <input type=hidden> are invisible so that they should be +// deleted silently. +["abc<audio>def</audio>[]ghi", + [["delete",""]], + "abghi", + [true], + {}], +["abc<input type=hidden>[]def", + [["delete",""]], + "abdef", + [true], + {}], ] diff --git a/testing/web-platform/tests/editing/data/forwarddelete.js b/testing/web-platform/tests/editing/data/forwarddelete.js @@ -3306,4 +3306,60 @@ var browserTests = [ "<div style=\"display:grid\"><span style=\"display:contents\"><span>abcdef</span></span></div>", [true], {}], + +// <object> content should be deleted if it has only fallback content. +["abc[]<object>def</object>", + [["forwarddelete",""]], + "abc<object>ef</object>", + [true], + {}], +["abc[]<object data=\"/images/green-1x1.png\">def</object>ghi", + [["forwarddelete",""]], + "abcghi", + [true], + {}], +["abc[]<object data=\"about:blank\">def</object>ghi", + [["forwarddelete",""]], + "abcghi", + [true], + {}], +// The following elements should be rendered with replaced content so that +// the element itself should be removed. +["abc[]<iframe>def</iframe>ghi", + [["forwarddelete",""]], + "abcghi", + [true], + {}], +["abc[]<meter>def</meter>ghi", + [["forwarddelete",""]], + "abcghi", + [true], + {}], +["abc[]<progress>def</progress>ghi", + [["forwarddelete",""]], + "abcghi", + [true], + {}], +["abc[]<audio controls>def</audio>ghi", + [["forwarddelete",""]], + "abcghi", + [true], + {}], +["abc[]<video controls>def</video>ghi", + [["forwarddelete",""]], + "abcghi", + [true], + {}], +// <audio> and <input type=hidden> are invisible so that they should be +// deleted silently. +["abc[]<audio>def</audio>ghi", + [["forwarddelete",""]], + "abchi", + [true], + {}], +["abc[]<input type=hidden>def", + [["forwarddelete",""]], + "abcef", + [true], + {}], ] diff --git a/testing/web-platform/tests/editing/other/editing-around-select-element.tentative.html b/testing/web-platform/tests/editing/other/editing-around-select-element.tentative.html @@ -18,7 +18,8 @@ const insertText = command === "insertText" ? "XYZ" : ""; * <option> and <optgroup>, but Selection API can do it (but browsers don't * show the result). In this case, any elements under `<select>` element * shouldn't be modified (deleted) for avoiding unexpected data loss for the - * users. + * users. However, if inserting text when selection range is entirely in the + * <select>, it's fine to insert text around the <select>. */ promise_test(async () => { @@ -32,7 +33,12 @@ function addPromiseTest(desc, initFunc, expectedResults) { initFunc(); document.execCommand(command, false, insertText); if (Array.isArray(expectedResults)) { - assert_in_array(document.body.innerHTML.replace(/(=""|<br>)/g, ""), expectedResults); + const uniqueArray = [...new Set(expectedResults)]; + if (uniqueArray.length > 1) { + assert_in_array(document.body.innerHTML.replace(/(=""|<br>)/g, ""), expectedResults); + } else { + assert_equals(document.body.innerHTML.replace(/(=""|<br>)/g, ""), expectedResults[0]); + } } else { assert_equals(document.body.innerHTML.replace(/(=""|<br>)/g, ""), expectedResults); } @@ -55,6 +61,8 @@ for (const multiple of ["", " multiple"]) { [ `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select></div>`, `<div contenteditable><p>ab${insertText}</p><select${multiple}><option>def</option></select></div>`, + // </p> is in the range so that <select> may be moved into the <p>. + `<div contenteditable><p>ab${insertText}<select${multiple}><option>def</option></select></p></div>`, ] ); @@ -68,7 +76,13 @@ for (const multiple of ["", " multiple"]) { 1 ); }, - `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select></div>`, + [ + `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select></div>`, + // If selection is treated as collapsed before <selection>, it's fine to insert text before <select>. + `<div contenteditable><p>abc</p>${insertText}<select${multiple}><option>def</option></select></div>`, + // If selection is treated as collapsed after <selection>, it's fine to insert text after <select>. + `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select>${insertText}</div>`, + ] ); addPromiseTest( @@ -86,6 +100,8 @@ for (const multiple of ["", " multiple"]) { [ `<div contenteditable><select${multiple}><option>abc</option></select><p>def</p></div>`, `<div contenteditable><select${multiple}><option>abc</option></select><p>${insertText}ef</p></div>`, + // <p> is in the range so that it's fine to move <select> into the <p>. + `<div contenteditable><select${multiple}><option>abc</option></select>${insertText}ef</div>`, ] ); @@ -99,7 +115,13 @@ for (const multiple of ["", " multiple"]) { 0 ); }, - `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select><p>ghi</p></div>` + [ + `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select><p>ghi</p></div>`, + // If selection is treated as collapsed before <selection>, it's fine to insert text before <select>. + `<div contenteditable><p>abc</p>${insertText}<select${multiple}><option>def</option></select><p>ghi</p></div>`, + // If selection is treated as collapsed after <selection>, it's fine to insert text after <select>. + `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select>${insertText}<p>ghi</p></div>`, + ] ); addPromiseTest( @@ -112,7 +134,13 @@ for (const multiple of ["", " multiple"]) { 1 ); }, - `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select><p>ghi</p></div>` + [ + `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select><p>ghi</p></div>`, + // If selection is treated as collapsed before <selection>, it's fine to insert text before <select>. + `<div contenteditable><p>abc</p>${insertText}<select${multiple}><option>def</option></select><p>ghi</p></div>`, + // If selection is treated as collapsed after <selection>, it's fine to insert text after <select>. + `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select>${insertText}<p>ghi</p></div>`, + ] ); addPromiseTest( @@ -124,7 +152,13 @@ for (const multiple of ["", " multiple"]) { document.querySelector("option") ); }, - `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select><p>ghi</p></div>` + [ + `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select><p>ghi</p></div>`, + // If selection is treated as collapsed before <selection>, it's fine to insert text before <select>. + `<div contenteditable><p>abc</p>${insertText}<select${multiple}><option>def</option></select><p>ghi</p></div>`, + // If selection is treated as collapsed after <selection>, it's fine to insert text after <select>. + `<div contenteditable><p>abc</p><select${multiple}><option>def</option></select>${insertText}<p>ghi</p></div>`, + ], ); addPromiseTest( @@ -139,7 +173,13 @@ for (const multiple of ["", " multiple"]) { 1, ); }, - `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>` + [ + `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>`, + // If selection is treated as collapsed before <selection>, it's fine to insert text before <select>. + `<div contenteditable><p>abc</p>${insertText}<select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>`, + // If selection is treated as collapsed after <selection>, it's fine to insert text after <select>. + `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select>${insertText}<p>jkl</p></div>`, + ] ); addPromiseTest( @@ -154,7 +194,13 @@ for (const multiple of ["", " multiple"]) { 1, ); }, - `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>` + [ + `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>`, + // If selection is treated as collapsed before <selection>, it's fine to insert text before <select>. + `<div contenteditable><p>abc</p>${insertText}<select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>`, + // If selection is treated as collapsed after <selection>, it's fine to insert text after <select>. + `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select>${insertText}<p>jkl</p></div>`, + ] ); addPromiseTest( @@ -169,7 +215,13 @@ for (const multiple of ["", " multiple"]) { 2, ); }, - `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>` + [ + `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>`, + // If selection is treated as collapsed before <selection>, it's fine to insert text before <select>. + `<div contenteditable><p>abc</p>${insertText}<select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>`, + // If selection is treated as collapsed after <selection>, it's fine to insert text after <select>. + `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select>${insertText}<p>jkl</p></div>`, + ] ); addPromiseTest( @@ -181,7 +233,13 @@ for (const multiple of ["", " multiple"]) { document.querySelector("select") ); }, - `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>` + [ + `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>`, + // If selection is treated as collapsed before <selection>, it's fine to insert text before <select>. + `<div contenteditable><p>abc</p>${insertText}<select${multiple}><option>def</option><option>ghi</option></select><p>jkl</p></div>`, + // If selection is treated as collapsed after <selection>, it's fine to insert text after <select>. + `<div contenteditable><p>abc</p><select${multiple}><option>def</option><option>ghi</option></select>${insertText}<p>jkl</p></div>`, + ] ); addPromiseTest( @@ -193,7 +251,13 @@ for (const multiple of ["", " multiple"]) { document.querySelector("optgroup") ); }, - `<div contenteditable><p>abc</p><select${multiple}><optgroup><option>def</option><option>ghi</option></optgroup></select><p>jkl</p></div>` + [ + `<div contenteditable><p>abc</p><select${multiple}><optgroup><option>def</option><option>ghi</option></optgroup></select><p>jkl</p></div>`, + // If selection is treated as collapsed before <selection>, it's fine to insert text before <select>. + `<div contenteditable><p>abc</p>${insertText}<select${multiple}><optgroup><option>def</option><option>ghi</option></optgroup></select><p>jkl</p></div>`, + // If selection is treated as collapsed after <selection>, it's fine to insert text after <select>. + `<div contenteditable><p>abc</p><select${multiple}><optgroup><option>def</option><option>ghi</option></optgroup></select>${insertText}<p>jkl</p></div>`, + ] ); addPromiseTest( @@ -209,7 +273,9 @@ for (const multiple of ["", " multiple"]) { ); }, [ + // XXX I think the first should be the best because <select> was between the <p>. `<div contenteditable><p>abc</p>${insertText}<p>jkl</p></div>`, + // XXX However, some browsers may want to normalize selection into the previous or next <p>. `<div contenteditable><p>abc${insertText}</p><p>jkl</p></div>`, `<div contenteditable><p>abc</p><p>${insertText}jkl</p></div>`, ] @@ -228,7 +294,9 @@ for (const multiple of ["", " multiple"]) { ); }, [ + // XXX I think the first should be the best because <select> was between the <p>. `<div contenteditable><p>abc</p>${insertText}<p>jkl</p></div>`, + // XXX However, some browsers may want to normalize selection into the previous or next <p>. `<div contenteditable><p>abc${insertText}</p><p>jkl</p></div>`, `<div contenteditable><p>abc</p><p>${insertText}jkl</p></div>`, ] @@ -243,7 +311,7 @@ for (const multiple of ["", " multiple"]) { document.querySelector("select") ); }, - `<select${multiple} contenteditable><option>abc</option><option>def</option></select>` + `<select${multiple} contenteditable><option>abc</option><option>def</option></select>`, ); addPromiseTest( @@ -292,6 +360,7 @@ addPromiseTest( document.querySelector("option") ); }, + // XXX It might be fine to modify <optgroup>/<option> not in <select> nor <datalist> if the browser does not replace its content. `<optgroup contenteditable><option>abc</option><option>def</option></optgroup>` ); @@ -304,6 +373,7 @@ addPromiseTest( document.querySelector("option") ); }, + // XXX It might be fine to modify <option> not in <select> nor <datalist> if the browser does not replace its content. `<option contenteditable>abc</option>` ); </script> diff --git a/testing/web-platform/tests/editing/other/insertparagraph-in-non-splittable-element.html b/testing/web-platform/tests/editing/other/insertparagraph-in-non-splittable-element.html @@ -81,7 +81,7 @@ const tests = [ initial: "<div><select><option>abc</option></select></div>", // <option> is a part of <select> and they are replaced so that it should // be handled outside of it. - expected: "<div><select><option>abc</option></select></div>", + expected: "<div><br></div><div><select><option>abc</option></select></div>", }, { selector: "progress", diff --git a/xpcom/base/ErrorList.py b/xpcom/base/ErrorList.py @@ -812,6 +812,11 @@ with modules["EDITOR"]: # non-collapsed range crosses editing host boundaries. errors["NS_ERROR_EDITOR_NO_EDITABLE_RANGE"] = FAILURE(4) + # An error code that indicates that there is no deletable selection ranges + # even though there are some editable ranges. E.g., if each editable range + # is in a replaced element or a void element. + errors["NS_ERROR_EDITOR_NO_DELETABLE_RANGE"] = FAILURE(5) + errors["NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND"] = SUCCESS(1) errors["NS_SUCCESS_EDITOR_FOUND_TARGET"] = SUCCESS(2)