Bug 1436295 - HTMLEditRules::WillInsertBreak() should cancel resetting EditorDOMPoint when HTMLEditRules::ReturnInParagraph() splits DOM node around the point r?m_kato
This patch only fixes warning, not changing actual behavior of editor.
HTMLEditRules::ReturnInParagraph() splits paragraph *around* given point.
Therefore, from point of view of caller, offset of setting point may be
invalid after HTMLEditRules::ReturnInParagraph() handled the edit action.
In this case, invalidating stored child of the point may cause warning
since offset may be larger than length of its container.
So, if HTMLEditRules::ReturnInParagraph() handles the edit action,
the caller, HTMLEditRules::WillInsertBreak(), should cancel invalidating
the stored child for avoiding unnecessary warning.
MozReview-Commit-ID: DKJlr0Awwlo
--- a/editor/libeditor/EditorDOMPoint.h
+++ b/editor/libeditor/EditorDOMPoint.h
@@ -825,48 +825,61 @@ ImplCycleCollectionTraverse(nsCycleColle
* destroyed. Additionally, users of this class can invalidate the offset
* manually when they need.
*/
class MOZ_STACK_CLASS AutoEditorDOMPointOffsetInvalidator final
{
public:
explicit AutoEditorDOMPointOffsetInvalidator(EditorDOMPoint& aPoint)
: mPoint(aPoint)
+ , mCanceled(false)
{
MOZ_ASSERT(aPoint.IsSetAndValid());
MOZ_ASSERT(mPoint.CanContainerHaveChildren());
mChild = mPoint.GetChild();
}
~AutoEditorDOMPointOffsetInvalidator()
{
- InvalidateOffset();
+ if (!mCanceled) {
+ InvalidateOffset();
+ }
}
/**
* Manually, invalidate offset of the given point.
*/
void InvalidateOffset()
{
if (mChild) {
mPoint.Set(mChild);
} else {
// If the point referred after the last child, let's keep referring
// after current last node of the old container.
mPoint.SetToEndOf(mPoint.GetContainer());
}
}
+ /**
+ * After calling Cancel(), mPoint won't be modified by the destructor.
+ */
+ void Cancel()
+ {
+ mCanceled = true;
+ }
+
private:
EditorDOMPoint& mPoint;
// Needs to store child node by ourselves because EditorDOMPoint stores
// child node with mRef which is previous sibling of current child node.
// Therefore, we cannot keep referring it if it's first child.
nsCOMPtr<nsIContent> mChild;
+ bool mCanceled;
+
AutoEditorDOMPointOffsetInvalidator() = delete;
AutoEditorDOMPointOffsetInvalidator(
const AutoEditorDOMPointOffsetInvalidator& aOther) = delete;
const AutoEditorDOMPointOffsetInvalidator& operator=(
const AutoEditorDOMPointOffsetInvalidator& aOther) = delete;
};
/**
@@ -879,37 +892,50 @@ private:
* Additionally, users of this class can invalidate the child manually when
* they need.
*/
class MOZ_STACK_CLASS AutoEditorDOMPointChildInvalidator final
{
public:
explicit AutoEditorDOMPointChildInvalidator(EditorDOMPoint& aPoint)
: mPoint(aPoint)
+ , mCanceled(false)
{
MOZ_ASSERT(aPoint.IsSetAndValid());
Unused << mPoint.Offset();
}
~AutoEditorDOMPointChildInvalidator()
{
- InvalidateChild();
+ if (!mCanceled) {
+ InvalidateChild();
+ }
}
/**
* Manually, invalidate child of the given point.
*/
void InvalidateChild()
{
mPoint.Set(mPoint.GetContainer(), mPoint.Offset());
}
+ /**
+ * After calling Cancel(), mPoint won't be modified by the destructor.
+ */
+ void Cancel()
+ {
+ mCanceled = true;
+ }
+
private:
EditorDOMPoint& mPoint;
+ bool mCanceled;
+
AutoEditorDOMPointChildInvalidator() = delete;
AutoEditorDOMPointChildInvalidator(
const AutoEditorDOMPointChildInvalidator& aOther) = delete;
const AutoEditorDOMPointChildInvalidator& operator=(
const AutoEditorDOMPointChildInvalidator& aOther) = delete;
};
} // namespace mozilla
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -1796,23 +1796,35 @@ HTMLEditRules::WillInsertBreak(Selection
AutoEditorDOMPointChildInvalidator lockOffset(atStartOfSelection);
// Paragraphs: special rules to look for <br>s
EditActionResult result = ReturnInParagraph(aSelection, *blockParent);
if (NS_WARN_IF(result.Failed())) {
return result.Rv();
}
*aHandled = result.Handled();
*aCancel = result.Canceled();
- // Fall through, we may not have handled it in ReturnInParagraph()
- }
-
- // If not already handled then do the standard thing
- if (!(*aHandled)) {
- *aHandled = true;
- return InsertBRElement(aSelection, atStartOfSelection);
+ if (result.Handled()) {
+ // Now, atStartOfSelection may be invalid because the left paragraph
+ // may have less children than its offset. For avoiding warnings of
+ // validation of EditorDOMPoint, we should not touch it anymore.
+ lockOffset.Cancel();
+ return NS_OK;
+ }
+ // Fall through, if ReturnInParagraph() didn't handle it.
+ MOZ_ASSERT(!*aCancel, "ReturnInParagraph canceled this edit action, "
+ "WillInsertBreak() needs to handle such case");
+ }
+
+ // If nobody handles this edit action, let's insert new <br> at the selection.
+ MOZ_ASSERT(!*aHandled, "Reached last resort of WillInsertBreak() "
+ "after the edit action is handled");
+ nsresult rv = InsertBRElement(aSelection, atStartOfSelection);
+ *aHandled = true;
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ return rv;
}
return NS_OK;
}
nsresult
HTMLEditRules::InsertBRElement(Selection& aSelection,
const EditorDOMPoint& aPointToBreak)
{