Bug 1411687 - part 2: Rewrite the check to insert a <br> element in HTMLEditRules::WillInsertBreak() r?m_kato
Currently, HTMLEditRules::WillInsertBreak() checks if the editing host can
contain a <p> element as a child or a descendant. However, this is not enough.
If an inline element has a block element which can contain a <p> element,
current implementation considers to insert a <br>. This is possible when
* The editing host is an unknown element including user defined element.
* The editing host is an inline element and its children and/or descendants
were added by JS. E.g., <span contenteditable> element can have <div>
element.
I think that we should consider to insert a <br> element when:
- There is no block ancestors in the editing host.
- The editing host is the only block element and it cannot contain <p> element
or the default paragraph separator is <br> element.
- The nearest block ancestor isn't a single-line container declared in the
execCommand spec and there are no block elements which can contain <p>
element.
Note that Chromium checks if CSS box of ancestors is block too. However,
it must be out of scope of this bug.
MozReview-Commit-ID: HdjU9t83Nd1
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -3538,54 +3538,54 @@ EditorBase::IsBlockNode(nsINode* aNode)
// screwing around with the class hierarchy here in order
// to not duplicate the code in GetNextNode/GetPrevNode
// across both EditorBase/HTMLEditor.
return false;
}
bool
EditorBase::CanContain(nsINode& aParent,
- nsIContent& aChild)
+ nsIContent& aChild) const
{
switch (aParent.NodeType()) {
case nsIDOMNode::ELEMENT_NODE:
case nsIDOMNode::DOCUMENT_FRAGMENT_NODE:
return TagCanContain(*aParent.NodeInfo()->NameAtom(), aChild);
}
return false;
}
bool
EditorBase::CanContainTag(nsINode& aParent,
- nsAtom& aChildTag)
+ nsAtom& aChildTag) const
{
switch (aParent.NodeType()) {
case nsIDOMNode::ELEMENT_NODE:
case nsIDOMNode::DOCUMENT_FRAGMENT_NODE:
return TagCanContainTag(*aParent.NodeInfo()->NameAtom(), aChildTag);
}
return false;
}
bool
EditorBase::TagCanContain(nsAtom& aParentTag,
- nsIContent& aChild)
+ nsIContent& aChild) const
{
switch (aChild.NodeType()) {
case nsIDOMNode::TEXT_NODE:
case nsIDOMNode::ELEMENT_NODE:
case nsIDOMNode::DOCUMENT_FRAGMENT_NODE:
return TagCanContainTag(aParentTag, *aChild.NodeInfo()->NameAtom());
}
return false;
}
bool
EditorBase::TagCanContainTag(nsAtom& aParentTag,
- nsAtom& aChildTag)
+ nsAtom& aChildTag) const
{
return true;
}
bool
EditorBase::IsRoot(nsIDOMNode* inNode)
{
NS_ENSURE_TRUE(inNode, false);
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -765,20 +765,20 @@ public:
static inline bool NodeIsType(nsIDOMNode* aNode, nsAtom* aTag)
{
return GetTag(aNode) == aTag;
}
/**
* Returns true if aParent can contain a child of type aTag.
*/
- bool CanContain(nsINode& aParent, nsIContent& aChild);
- bool CanContainTag(nsINode& aParent, nsAtom& aTag);
- bool TagCanContain(nsAtom& aParentTag, nsIContent& aChild);
- virtual bool TagCanContainTag(nsAtom& aParentTag, nsAtom& aChildTag);
+ bool CanContain(nsINode& aParent, nsIContent& aChild) const;
+ bool CanContainTag(nsINode& aParent, nsAtom& aTag) const;
+ bool TagCanContain(nsAtom& aParentTag, nsIContent& aChild) const;
+ virtual bool TagCanContainTag(nsAtom& aParentTag, nsAtom& aChildTag) const;
/**
* Returns true if aNode is our root node.
*/
bool IsRoot(nsIDOMNode* inNode);
bool IsRoot(nsINode* inNode);
bool IsEditorRoot(nsINode* aNode);
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -1532,16 +1532,45 @@ HTMLEditRules::WillLoadHTML(Selection* a
}
mHTMLEditor->DeleteNode(mBogusNode);
mBogusNode = nullptr;
}
return NS_OK;
}
+bool
+HTMLEditRules::CanContainParagraph(Element& aElement) const
+{
+ if (NS_WARN_IF(!mHTMLEditor)) {
+ return false;
+ }
+
+ if (mHTMLEditor->CanContainTag(aElement, *nsGkAtoms::p)) {
+ return true;
+ }
+
+ // Even if the element cannot have a <p> element as a child, it can contain
+ // <p> element as a descendant if it's one of the following elements.
+ if (aElement.IsAnyOfHTMLElements(nsGkAtoms::ol,
+ nsGkAtoms::ul,
+ nsGkAtoms::dl,
+ nsGkAtoms::table,
+ nsGkAtoms::thead,
+ nsGkAtoms::tbody,
+ nsGkAtoms::tfoot,
+ nsGkAtoms::tr)) {
+ return true;
+ }
+
+ // XXX Otherwise, Chromium checks the CSS box is a block, but we don't do it
+ // for now.
+ return false;
+}
+
nsresult
HTMLEditRules::WillInsertBreak(Selection& aSelection,
bool* aCancel,
bool* aHandled)
{
MOZ_ASSERT(aCancel && aHandled);
*aCancel = false;
*aHandled = false;
@@ -1580,64 +1609,91 @@ HTMLEditRules::WillInsertBreak(Selection
int32_t offset = aSelection.GetRangeAt(0)->StartOffset();
// Do nothing if the node is read-only
if (!htmlEditor->IsModifiableNode(node)) {
*aCancel = true;
return NS_OK;
}
- // Identify the block
- nsCOMPtr<Element> blockParent = htmlEditor->GetBlock(node);
- NS_ENSURE_TRUE(blockParent, NS_ERROR_FAILURE);
-
// If the active editing host is an inline element, or if the active editing
// host is the block parent itself and we're configured to use <br> as a
// paragraph separator, just append a <br>.
nsCOMPtr<Element> host = htmlEditor->GetActiveEditingHost();
if (NS_WARN_IF(!host)) {
return NS_ERROR_FAILURE;
}
- ParagraphSeparator separator = mHTMLEditor->GetDefaultParagraphSeparator();
- if (!IsBlockNode(*host) ||
- // The nodes that can contain p and div are the same. If the editing
- // host is a <p> or similar, we have to just insert a newline.
- (!mHTMLEditor->CanContainTag(*host, *nsGkAtoms::p) &&
- // These can't contain <p> as a child, but can as a descendant, so we
- // don't have to fall back to inserting a newline.
- !host->IsAnyOfHTMLElements(nsGkAtoms::ol, nsGkAtoms::ul, nsGkAtoms::dl,
- nsGkAtoms::table, nsGkAtoms::thead,
- nsGkAtoms::tbody, nsGkAtoms::tfoot,
- nsGkAtoms::tr)) ||
- (host == blockParent && separator == ParagraphSeparator::br)) {
+
+ // Look for the nearest parent block. However, don't return error even if
+ // there is no block parent here because in such case, i.e., editing host
+ // is an inline element, we should insert <br> simply.
+ RefPtr<Element> blockParent = HTMLEditor::GetBlock(node, host);
+
+ ParagraphSeparator separator = htmlEditor->GetDefaultParagraphSeparator();
+ bool insertBRElement;
+ // If there is no block parent in the editing host, i.e., the editing host
+ // itself is also a non-block element, we should insert a <br> element.
+ if (!blockParent) {
+ // XXX Chromium checks if the CSS box of the editing host is block.
+ insertBRElement = true;
+ }
+ // If only the editing host is block, and the default paragraph separator
+ // is <br> or the editing host cannot contain a <p> element, we should
+ // insert a <br> element.
+ else if (host == blockParent) {
+ insertBRElement =
+ separator == ParagraphSeparator::br || !CanContainParagraph(*host);
+ }
+ // If the nearest block parent is a single-line container declared in
+ // the execCommand spec and not the editing host, we should separate the
+ // block even if the default paragraph separator is <br> element.
+ else if (HTMLEditUtils::IsSingleLineContainer(*blockParent)) {
+ insertBRElement = false;
+ }
+ // Otherwise, unless there is no block ancestor which can contain <p>
+ // element, we shouldn't insert a <br> element here.
+ else {
+ insertBRElement = true;
+ for (Element* blockAncestor = blockParent;
+ blockAncestor && insertBRElement;
+ blockAncestor = HTMLEditor::GetBlockNodeParent(blockAncestor, host)) {
+ insertBRElement = !CanContainParagraph(*blockAncestor);
+ }
+ }
+
+ // If we cannot insert a <p>/<div> element at the selection, we should insert
+ // a <br> element instead.
+ if (insertBRElement) {
nsresult rv = StandardBreakImpl(node, offset, aSelection);
NS_ENSURE_SUCCESS(rv, rv);
*aHandled = true;
return NS_OK;
}
+
if (host == blockParent && separator != ParagraphSeparator::br) {
// Insert a new block first
MOZ_ASSERT(separator == ParagraphSeparator::div ||
separator == ParagraphSeparator::p);
nsresult rv = MakeBasicBlock(aSelection,
ParagraphSeparatorElement(separator));
// We warn on failure, but don't handle it, because it might be harmless.
// Instead we just check that a new block was actually created.
- Unused << NS_WARN_IF(NS_FAILED(rv));
+ NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+ "HTMLEditRules::MakeBasicBlock() failed");
// Reinitialize node/offset in case they're not inside the new block
if (NS_WARN_IF(!aSelection.GetRangeAt(0) ||
!aSelection.GetRangeAt(0)->GetStartContainer())) {
return NS_ERROR_FAILURE;
}
node = *aSelection.GetRangeAt(0)->GetStartContainer();
child = aSelection.GetRangeAt(0)->GetChildAtStartOffset();
offset = aSelection.GetRangeAt(0)->StartOffset();
- blockParent = mHTMLEditor->GetBlock(node);
+ blockParent = mHTMLEditor->GetBlock(node, host);
if (NS_WARN_IF(!blockParent)) {
return NS_ERROR_UNEXPECTED;
}
if (NS_WARN_IF(blockParent == host)) {
// Didn't create a new block for some reason, fall back to <br>
rv = StandardBreakImpl(node, offset, aSelection);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -168,16 +168,22 @@ protected:
nsIEditor::EStripWrappers aStripWrappers,
bool* aCancel, bool* aHandled);
nsresult DidDeleteSelection(Selection* aSelection,
nsIEditor::EDirection aDir,
nsresult aResult);
nsresult InsertBRIfNeeded(Selection* aSelection);
/**
+ * CanContainParagraph() returns true if aElement can have a <p> element as
+ * its child or its descendant.
+ */
+ bool CanContainParagraph(Element& aElement) const;
+
+ /**
* Insert a normal <br> element or a moz-<br> element to aNode when
* aNode is a block and it has no children.
*
* @param aNode Reference to a block parent.
* @param aInsertMozBR true if this should insert a moz-<br> element.
* Otherwise, i.e., this should insert a normal <br>
* element, false.
*/
--- a/editor/libeditor/HTMLEditUtils.cpp
+++ b/editor/libeditor/HTMLEditUtils.cpp
@@ -829,9 +829,35 @@ bool
HTMLEditUtils::IsContainer(int32_t aTag)
{
NS_ASSERTION(aTag > eHTMLTag_unknown && aTag <= eHTMLTag_userdefined,
"aTag out of range!");
return kElements[aTag - 1].mIsContainer;
}
+bool
+HTMLEditUtils::IsNonListSingleLineContainer(nsINode& aNode)
+{
+ return aNode.IsAnyOfHTMLElements(nsGkAtoms::address,
+ nsGkAtoms::div,
+ nsGkAtoms::h1,
+ nsGkAtoms::h2,
+ nsGkAtoms::h3,
+ nsGkAtoms::h4,
+ nsGkAtoms::h5,
+ nsGkAtoms::h6,
+ nsGkAtoms::listing,
+ nsGkAtoms::p,
+ nsGkAtoms::pre,
+ nsGkAtoms::xmp);
+}
+
+bool
+HTMLEditUtils::IsSingleLineContainer(nsINode& aNode)
+{
+ return IsNonListSingleLineContainer(aNode) ||
+ aNode.IsAnyOfHTMLElements(nsGkAtoms::li,
+ nsGkAtoms::dt,
+ nsGkAtoms::dd);
+}
+
} // namespace mozilla
--- a/editor/libeditor/HTMLEditUtils.h
+++ b/editor/libeditor/HTMLEditUtils.h
@@ -11,17 +11,16 @@
class nsIDOMNode;
class nsINode;
namespace mozilla {
class HTMLEditUtils final
{
public:
- // from nsHTMLEditRules:
static bool IsInlineStyle(nsINode* aNode);
static bool IsInlineStyle(nsIDOMNode *aNode);
static bool IsFormatNode(nsINode* aNode);
static bool IsFormatNode(nsIDOMNode* aNode);
static bool IsNodeThatCanOutdent(nsIDOMNode* aNode);
static bool IsHeader(nsINode& aNode);
static bool IsHeader(nsIDOMNode* aNode);
static bool IsParagraph(nsIDOMNode* aNode);
@@ -55,14 +54,22 @@ public:
static bool IsMozDiv(nsIDOMNode* aNode);
static bool IsMailCite(nsINode* aNode);
static bool IsMailCite(nsIDOMNode* aNode);
static bool IsFormWidget(nsINode* aNode);
static bool IsFormWidget(nsIDOMNode* aNode);
static bool SupportsAlignAttr(nsINode& aNode);
static bool CanContain(int32_t aParent, int32_t aChild);
static bool IsContainer(int32_t aTag);
+
+ /**
+ * See execCommand spec:
+ * https://w3c.github.io/editing/execCommand.html#non-list-single-line-container
+ * https://w3c.github.io/editing/execCommand.html#single-line-container
+ */
+ static bool IsNonListSingleLineContainer(nsINode& aNode);
+ static bool IsSingleLineContainer(nsINode& aNode);
};
} // namespace mozilla
#endif // #ifndef HTMLEditUtils_h
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -3466,17 +3466,17 @@ HTMLEditor::EndOperation()
// post processing
nsresult rv = rules ? rules->AfterEdit(mAction, mDirection) : NS_OK;
EditorBase::EndOperation(); // will clear mAction, mDirection
return rv;
}
bool
HTMLEditor::TagCanContainTag(nsAtom& aParentTag,
- nsAtom& aChildTag)
+ nsAtom& aChildTag) const
{
int32_t childTagEnum;
// XXX Should this handle #cdata-section too?
if (&aChildTag == nsGkAtoms::textTagName) {
childTagEnum = eHTMLTag_text;
} else {
childTagEnum = nsHTMLTags::AtomTagToId(&aChildTag);
}
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -289,17 +289,17 @@ public:
* with a call to EndOperation.
*/
NS_IMETHOD EndOperation() override;
/**
* returns true if aParentTag can contain a child of type aChildTag.
*/
virtual bool TagCanContainTag(nsAtom& aParentTag,
- nsAtom& aChildTag) override;
+ nsAtom& aChildTag) const override;
/**
* Returns true if aNode is a container.
*/
virtual bool IsContainer(nsINode* aNode) override;
virtual bool IsContainer(nsIDOMNode* aNode) override;
/**