Bug 1408227 - part 2: WSRunObject::InsertBreak() should take |const EditorRawDOMPoint&| as an argument r?m_kato
WSRunObject::InsertBreak() should take |const EditorRawDOMPoint&| instead of
a pair of container node and offset in it.
MozReview-Commit-ID: 38OAn4dvR7x
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -1504,30 +1504,39 @@ HTMLEditRules::WillInsertText(EditAction
return rv;
}
pos++;
currentPoint = pointAfterInsertedSpaces;
pointToInsert = pointAfterInsertedSpaces;
}
// is it a return?
else if (subStr.Equals(newlineStr)) {
- nsCOMPtr<nsINode> curNode = currentPoint.Container();
- int32_t curOffset = currentPoint.Offset();
- nsCOMPtr<Element> br = wsObj.InsertBreak(address_of(curNode),
- &curOffset,
- nsIEditor::eNone);
- NS_ENSURE_TRUE(br, NS_ERROR_FAILURE);
+ RefPtr<Element> newBRElement =
+ wsObj.InsertBreak(*aSelection, currentPoint.AsRaw(),
+ nsIEditor::eNone);
+ if (NS_WARN_IF(!newBRElement)) {
+ return NS_ERROR_FAILURE;
+ }
pos++;
- if (br->GetNextSibling()) {
- pointToInsert.Set(br->GetNextSibling());
+ if (newBRElement->GetNextSibling()) {
+ pointToInsert.Set(newBRElement->GetNextSibling());
} else {
- pointToInsert.Set(curNode, curNode->Length());
+ pointToInsert.Set(currentPoint.Container(),
+ currentPoint.Container()->Length());
}
- currentPoint.Set(curNode, curOffset);
- MOZ_ASSERT(currentPoint == pointToInsert);
+ currentPoint.Set(newBRElement);
+ DebugOnly<bool> advanced = currentPoint.AdvanceOffset();
+ NS_WARNING_ASSERTION(advanced,
+ "Failed to advance offset to after the new <br> node");
+ // XXX If the newBRElement has been moved or removed by mutation
+ // observer, we hit this assert. We need to check if
+ // newBRElement is in expected point, though, we must have
+ // a lot of same bugs...
+ NS_WARNING_ASSERTION(currentPoint == pointToInsert,
+ "Perhaps, newBRElement has been moved or removed unexpectedly");
} else {
EditorRawDOMPoint pointAfterInsertedString;
rv = wsObj.InsertText(*doc, subStr, currentPoint.AsRaw(),
&pointAfterInsertedString);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
currentPoint = pointAfterInsertedString;
@@ -1868,18 +1877,22 @@ HTMLEditRules::StandardBreakImpl(nsINode
SplitAtEdges::eDoNotCreateEmptyContainer);
if (NS_WARN_IF(splitLinkNodeResult.Failed())) {
return splitLinkNodeResult.Rv();
}
EditorRawDOMPoint splitPoint(splitLinkNodeResult.SplitPoint());
node = splitPoint.Container();
aOffset = splitPoint.Offset();
}
- brNode = wsObj.InsertBreak(address_of(node), &aOffset, nsIEditor::eNone);
- NS_ENSURE_TRUE(brNode, NS_ERROR_FAILURE);
+ brNode =
+ wsObj.InsertBreak(aSelection, EditorRawDOMPoint(node, aOffset),
+ nsIEditor::eNone);
+ if (NS_WARN_IF(!brNode)) {
+ return NS_ERROR_FAILURE;
+ }
}
node = brNode->GetParentNode();
NS_ENSURE_TRUE(node, NS_ERROR_NULL_POINTER);
if (bAfterBlock && bBeforeBlock) {
// We just placed a br between block boundaries. This is the one case
// where we want the selection to be before the br we just placed, as the
// br will be on a new line, rather than at end of prior line.
aSelection.SetInterlinePosition(true);
--- a/editor/libeditor/WSRunObject.cpp
+++ b/editor/libeditor/WSRunObject.cpp
@@ -163,49 +163,56 @@ WSRunObject::PrepareToSplitAcrossBlocks(
aSplitNode, aSplitOffset);
WSRunObject wsObj(aHTMLEditor, *aSplitNode, *aSplitOffset);
return wsObj.PrepareToSplitAcrossBlocksPriv();
}
already_AddRefed<Element>
-WSRunObject::InsertBreak(nsCOMPtr<nsINode>* aInOutParent,
- int32_t* aInOutOffset,
+WSRunObject::InsertBreak(Selection& aSelection,
+ const EditorRawDOMPoint& aPointToInsert,
nsIEditor::EDirection aSelect)
{
+ if (NS_WARN_IF(!aPointToInsert.IsSet())) {
+ return nullptr;
+ }
+
// MOOSE: for now, we always assume non-PRE formatting. Fix this later.
// meanwhile, the pre case is handled in WillInsertText in
// HTMLEditRules.cpp
- NS_ENSURE_TRUE(aInOutParent && aInOutOffset, nullptr);
WSFragment *beforeRun, *afterRun;
- FindRun(*aInOutParent, *aInOutOffset, &beforeRun, false);
- FindRun(*aInOutParent, *aInOutOffset, &afterRun, true);
+ FindRun(aPointToInsert.Container(), aPointToInsert.Offset(),
+ &beforeRun, false);
+ FindRun(aPointToInsert.Container(), aPointToInsert.Offset(),
+ &afterRun, true);
+ EditorDOMPoint pointToInsert(aPointToInsert);
{
// Some scoping for AutoTrackDOMPoint. This will track our insertion
// point while we tweak any surrounding whitespace
- AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater, aInOutParent,
- aInOutOffset);
+ AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater, &pointToInsert);
// Handle any changes needed to ws run after inserted br
if (!afterRun || (afterRun->mType & WSType::trailingWS)) {
// Don't need to do anything. Just insert break. ws won't change.
} else if (afterRun->mType & WSType::leadingWS) {
// Delete the leading ws that is after insertion point. We don't
// have to (it would still not be significant after br), but it's
// just more aesthetically pleasing to.
- nsresult rv = DeleteChars(*aInOutParent, *aInOutOffset,
- afterRun->mEndNode, afterRun->mEndOffset);
+ nsresult rv =
+ DeleteChars(pointToInsert.Container(), pointToInsert.Offset(),
+ afterRun->mEndNode, afterRun->mEndOffset);
NS_ENSURE_SUCCESS(rv, nullptr);
} else if (afterRun->mType == WSType::normalWS) {
// Need to determine if break at front of non-nbsp run. If so, convert
// run to nbsp.
- WSPoint thePoint = GetCharAfter(*aInOutParent, *aInOutOffset);
+ WSPoint thePoint =
+ GetCharAfter(pointToInsert.Container(), pointToInsert.Offset());
if (thePoint.mTextNode && nsCRT::IsAsciiSpace(thePoint.mChar)) {
WSPoint prevPoint = GetCharBefore(thePoint);
if (!prevPoint.mTextNode ||
(prevPoint.mTextNode && !nsCRT::IsAsciiSpace(prevPoint.mChar))) {
// We are at start of non-nbsps. Convert to a single nbsp.
nsresult rv = ConvertToNBSP(thePoint);
NS_ENSURE_SUCCESS(rv, nullptr);
}
@@ -213,43 +220,34 @@ WSRunObject::InsertBreak(nsCOMPtr<nsINod
}
// Handle any changes needed to ws run before inserted br
if (!beforeRun || (beforeRun->mType & WSType::leadingWS)) {
// Don't need to do anything. Just insert break. ws won't change.
} else if (beforeRun->mType & WSType::trailingWS) {
// Need to delete the trailing ws that is before insertion point, because it
// would become significant after break inserted.
- nsresult rv = DeleteChars(beforeRun->mStartNode, beforeRun->mStartOffset,
- *aInOutParent, *aInOutOffset);
+ nsresult rv =
+ DeleteChars(beforeRun->mStartNode, beforeRun->mStartOffset,
+ pointToInsert.Container(), pointToInsert.Offset());
NS_ENSURE_SUCCESS(rv, nullptr);
} else if (beforeRun->mType == WSType::normalWS) {
// Try to change an nbsp to a space, just to prevent nbsp proliferation
- nsresult rv = CheckTrailingNBSP(beforeRun, *aInOutParent, *aInOutOffset);
+ nsresult rv =
+ CheckTrailingNBSP(beforeRun, pointToInsert.Container(),
+ pointToInsert.Offset());
NS_ENSURE_SUCCESS(rv, nullptr);
}
}
- RefPtr<Selection> selection = mHTMLEditor->GetSelection();
- if (NS_WARN_IF(!selection)) {
- return nullptr;
- }
RefPtr<Element> newBRElement =
- mHTMLEditor->CreateBRImpl(*selection,
- EditorRawDOMPoint(*aInOutParent, *aInOutOffset),
- aSelect);
+ mHTMLEditor->CreateBRImpl(aSelection, pointToInsert.AsRaw(), aSelect);
if (NS_WARN_IF(!newBRElement)) {
return nullptr;
}
- EditorRawDOMPoint atNewBRElement(newBRElement);
- DebugOnly<bool> advanced = atNewBRElement.AdvanceOffset();
- NS_WARNING_ASSERTION(advanced,
- "Failed to advance offset to after the new <br> element");
- *aInOutParent = atNewBRElement.Container();
- *aInOutOffset = atNewBRElement.Offset();
return newBRElement.forget();
}
nsresult
WSRunObject::InsertText(nsIDocument& aDocument,
const nsAString& aStringToInsert,
const EditorRawDOMPoint& aPointToInsert,
EditorRawDOMPoint* aPointAfterInsertedString)
--- a/editor/libeditor/WSRunObject.h
+++ b/editor/libeditor/WSRunObject.h
@@ -206,23 +206,39 @@ public:
// {aSplitNode,aSplitOffset} in preparation for a block parent to be split.
// Note that the aSplitNode and aSplitOffset are adjusted in response to
// any DOM changes we make while adjusting ws. Example of fixup: normalws
// before {aSplitNode,aSplitOffset} needs to end with nbsp.
static nsresult PrepareToSplitAcrossBlocks(HTMLEditor* aHTMLEditor,
nsCOMPtr<nsINode>* aSplitNode,
int32_t* aSplitOffset);
- // InsertBreak inserts a br node at {aInOutParent,aInOutOffset}
- // and makes any needed adjustments to ws around that point.
- // example of fixup: normalws after {aInOutParent,aInOutOffset}
- // needs to begin with nbsp.
- already_AddRefed<dom::Element> InsertBreak(nsCOMPtr<nsINode>* aInOutParent,
- int32_t* aInOutOffset,
- nsIEditor::EDirection aSelect);
+ /**
+ * InsertBreak() inserts a <br> node at (before) aPointToInsert and delete
+ * unnecessary whitespaces around there and/or replaces whitespaces with
+ * non-breaking spaces. Note that if the point is in a text node, the
+ * text node will be split and insert new <br> node between the left node
+ * and the right node.
+ *
+ * @param aSelection The selection for the editor.
+ * @param aPointToInsert The point to insert new <br> element. Note that
+ * it'll be inserted before this point. I.e., the
+ * point will be the point of new <br>.
+ * @param aSelect If eNone, this won't change selection.
+ * If eNext, selection will be collapsed after the
+ * <br> element.
+ * If ePrevious, selection will be collapsed at the
+ * <br> element.
+ * @return The new <br> node. If failed to create new <br>
+ * node, returns nullptr.
+ */
+ already_AddRefed<dom::Element>
+ InsertBreak(Selection& aSelection,
+ const EditorRawDOMPoint& aPointToInsert,
+ nsIEditor::EDirection aSelect);
/**
* InsertTextImpl() inserts aStringToInsert to aPointToInsert and makes any
* needed adjustments to white spaces around that point. E.g., trailing white
* spaces before aPointToInsert needs to be removed.
* This calls EditorBase::InsertTextImpl() after adjusting white spaces.
* So, please refer the method's explanation to know what this method exactly
* does.