Bug 1415800 - part 5: Redesign HTMLEditRules::FindNearSelectableNode() r?m_kato
First, the method name is not correct. It tries to find an editable node near
the given DOM point. Therefore, it should be FindNearEditableNode().
Next, the implementation did something odd. E.g., in the first |if| block,
when |nearNode| is nullptr, it returns nullptr. However, following |if| block
does something only when |nearNode| is nullptr. So, we can get rid of the
second |if| block. Then, nobody will change aDirection. So, we can make it
not a reference now.
Similarly, in |while| block, if |nearNode| becomes nullptr, it returns error.
However, following block checks if |nearNode| is NOT nullptr. So, we can get
rid of this |if| statement and outdent its block.
Additionally, |curNode| isn't necessary. It only increments the refcount
redundantly. So, we can get rid of it.
Finally, FindNearEditableNode() can return found node directly instead of
error code because error code doesn't make sense. Not found an editable
node is not illegal. And also it can take EditorRawDOMPoint instead of
a set of container, child and offset of the child in the container.
MozReview-Commit-ID: CTI581PhJMd
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -7984,118 +7984,84 @@ HTMLEditRules::AdjustSelection(Selection
EditorBase::IsTextNode(nearNode) ||
nearNode->IsAnyOfHTMLElements(nsGkAtoms::img,
nsGkAtoms::hr))) {
return NS_OK; // this is a good place for the caret to be
}
// look for a nearby text node.
// prefer the correct direction.
- nsresult rv = FindNearSelectableNode(point.Container(), point.Offset(),
- point.GetChildAtOffset(), aAction,
- address_of(nearNode));
- NS_ENSURE_SUCCESS(rv, rv);
-
+ nearNode = FindNearEditableNode(point.AsRaw(), aAction);
if (!nearNode) {
return NS_OK;
}
+
EditorDOMPoint pt = GetGoodSelPointForNode(*nearNode, aAction);
- rv = aSelection->Collapse(pt.AsRaw());
- if (NS_WARN_IF(NS_FAILED(rv))) {
- return rv;
+ ErrorResult error;
+ aSelection->Collapse(pt.AsRaw(), error);
+ if (NS_WARN_IF(error.Failed())) {
+ return error.StealNSResult();
}
return NS_OK;
}
-
-nsresult
-HTMLEditRules::FindNearSelectableNode(nsINode* aSelNode,
- int32_t aSelOffset,
- nsINode* aChildAtOffset,
- nsIEditor::EDirection& aDirection,
- nsCOMPtr<nsIContent>* outSelectableNode)
-{
- NS_ENSURE_TRUE(aSelNode && outSelectableNode, NS_ERROR_NULL_POINTER);
- *outSelectableNode = nullptr;
-
- EditorRawDOMPoint point(aSelNode,
- aChildAtOffset && aChildAtOffset->IsContent() ?
- aChildAtOffset->AsContent() : nullptr,
- aSelOffset);
-
- nsCOMPtr<nsIContent> nearNode, curNode;
+nsIContent*
+HTMLEditRules::FindNearEditableNode(const EditorRawDOMPoint& aPoint,
+ nsIEditor::EDirection aDirection)
+{
+ if (NS_WARN_IF(!aPoint.IsSet()) ||
+ NS_WARN_IF(!mHTMLEditor)) {
+ return nullptr;
+ }
+ MOZ_ASSERT(aPoint.IsSetAndValid());
+
+ RefPtr<HTMLEditor> htmlEditor(mHTMLEditor);
+
+ nsIContent* nearNode = nullptr;
if (aDirection == nsIEditor::ePrevious) {
- NS_ENSURE_STATE(mHTMLEditor);
- nearNode = mHTMLEditor->GetPreviousEditableHTMLNode(point);
- if (NS_WARN_IF(!nearNode)) {
- return NS_ERROR_FAILURE;
+ nearNode = htmlEditor->GetPreviousEditableHTMLNode(aPoint);
+ if (!nearNode) {
+ return nullptr; // Not illegal.
}
} else {
- NS_ENSURE_STATE(mHTMLEditor);
- nearNode = mHTMLEditor->GetNextEditableHTMLNode(point);
+ nearNode = htmlEditor->GetNextEditableHTMLNode(aPoint);
if (NS_WARN_IF(!nearNode)) {
- return NS_ERROR_FAILURE;
- }
- }
-
- // Try the other direction then.
- if (!nearNode) {
- if (aDirection == nsIEditor::ePrevious) {
- aDirection = nsIEditor::eNext;
- } else {
- aDirection = nsIEditor::ePrevious;
- }
-
- if (aDirection == nsIEditor::ePrevious) {
- NS_ENSURE_STATE(mHTMLEditor);
- nearNode = mHTMLEditor->GetPreviousEditableHTMLNode(point);
- if (NS_WARN_IF(!nearNode)) {
- return NS_ERROR_FAILURE;
- }
- } else {
- NS_ENSURE_STATE(mHTMLEditor);
- nearNode = mHTMLEditor->GetPreviousEditableHTMLNode(point);
- if (NS_WARN_IF(!nearNode)) {
- return NS_ERROR_FAILURE;
- }
+ // Perhaps, illegal because the node pointed by aPoint isn't editable
+ // and nobody of previous nodes is editable.
+ return nullptr;
}
}
// scan in the right direction until we find an eligible text node,
// but don't cross any breaks, images, or table elements.
+ // XXX This comment sounds odd. |nearNode| may have already crossed breaks
+ // and/or images.
while (nearNode && !(EditorBase::IsTextNode(nearNode) ||
TextEditUtils::IsBreak(nearNode) ||
HTMLEditUtils::IsImage(nearNode))) {
- curNode = nearNode;
if (aDirection == nsIEditor::ePrevious) {
- NS_ENSURE_STATE(mHTMLEditor);
- nearNode = mHTMLEditor->GetPreviousEditableHTMLNode(*curNode);
+ nearNode = htmlEditor->GetPreviousEditableHTMLNode(*nearNode);
if (NS_WARN_IF(!nearNode)) {
- return NS_ERROR_FAILURE;
+ return nullptr;
}
} else {
- NS_ENSURE_STATE(mHTMLEditor);
- nearNode = mHTMLEditor->GetNextEditableHTMLNode(*curNode);
+ nearNode = htmlEditor->GetNextEditableHTMLNode(*nearNode);
if (NS_WARN_IF(!nearNode)) {
- return NS_ERROR_FAILURE;
- }
- }
- NS_ENSURE_STATE(mHTMLEditor);
- }
-
- if (nearNode) {
- // don't cross any table elements
- if (InDifferentTableElements(nearNode, aSelNode)) {
- return NS_OK;
- }
-
- // otherwise, ok, we have found a good spot to put the selection
- *outSelectableNode = nearNode;
- }
- return NS_OK;
+ return nullptr;
+ }
+ }
+ }
+
+ // don't cross any table elements
+ if (InDifferentTableElements(nearNode, aPoint.Container())) {
+ return nullptr;
+ }
+
+ // otherwise, ok, we have found a good spot to put the selection
+ return nearNode;
}
bool
HTMLEditRules::InDifferentTableElements(nsIDOMNode* aNode1,
nsIDOMNode* aNode2)
{
nsCOMPtr<nsINode> node1 = do_QueryInterface(aNode1);
nsCOMPtr<nsINode> node2 = do_QueryInterface(aNode2);
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -405,21 +405,32 @@ protected:
nsresult ReapplyCachedStyles();
void ClearCachedStyles();
void AdjustSpecialBreaks();
nsresult AdjustWhitespace(Selection* aSelection);
nsresult PinSelectionToNewBlock(Selection* aSelection);
void CheckInterlinePosition(Selection& aSelection);
nsresult AdjustSelection(Selection* aSelection,
nsIEditor::EDirection aAction);
- nsresult FindNearSelectableNode(nsINode* aSelNode,
- int32_t aSelOffset,
- nsINode* aChildAtOffset,
- nsIEditor::EDirection& aDirection,
- nsCOMPtr<nsIContent>* outSelectableNode);
+
+ /**
+ * FindNearEditableNode() tries to find an editable node near aPoint.
+ *
+ * @param aPoint The DOM point where to start to search from.
+ * @param aDirection If nsIEditor::ePrevious is set, this searches an
+ * editable node from next nodes. Otherwise, from
+ * previous nodes.
+ * @return If found, returns non-nullptr. Otherwise, nullptr.
+ * Note that if found node is in different table element,
+ * this returns nullptr.
+ * And also if aDirection is not nsIEditor::ePrevious,
+ * the result may be the node pointed by aPoint.
+ */
+ nsIContent* FindNearEditableNode(const EditorRawDOMPoint& aPoint,
+ nsIEditor::EDirection aDirection);
/**
* Returns true if aNode1 or aNode2 or both is the descendant of some type of
* table element, but their nearest table element ancestors differ. "Table
* element" here includes not just <table> but also <td>, <tbody>, <tr>, etc.
* The nodes count as being their own descendants for this purpose, so a
* table element is its own nearest table element ancestor.
*/
bool InDifferentTableElements(nsIDOMNode* aNode1, nsIDOMNode* aNode2);