1406577 - Rewrite HTMLEditRules::FindNearSelectableNode() to use internal DOM APIs

Status: RESOLVED FIXED

(Core :: DOM: Editor, enhancement)

Comment 1

[(no longer active)]

Attachment: Part 1: Rewrite HTMLEditRules::FindNearSelectableNode() to use internal DOM APIs

Comment 2

[(no longer active)]

Attachment: Part 2: Remove some dead code

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8916184 [details] [diff] [review]
Part 1: Rewrite HTMLEditRules::FindNearSelectableNode() to use internal DOM APIs

>@@ -7568,17 +7568,17 @@ HTMLEditRules::AdjustSelection(Selection* aSelection,
>   }
> 
>   // do we need to insert a special mozBR?  We do if we are:
>   // 1) prior node is in same block where selection is AND
>   // 2) prior node is a br AND
>   // 3) that br is not visible
> 
>   NS_ENSURE_STATE(mHTMLEditor);
>-  nsCOMPtr<nsIContent> nearNode =
>+  nsCOMPtr<nsINode> nearNode =
>     mHTMLEditor->GetPriorHTMLNode(selNode, selOffset);

This change is really sad.  However, do we really need to do this?

GetPriorHTMLNode() returns nsIContent.

>@@ -7627,116 +7627,113 @@ HTMLEditRules::AdjustSelection(Selection* aSelection,
>                    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.
>-  nsCOMPtr<nsIDOMNode> nearNodeDOM = GetAsDOMNode(nearNode);
>-  rv = FindNearSelectableNode(GetAsDOMNode(selNode), selOffset, aAction,
>-                              address_of(nearNodeDOM));
>+  rv = FindNearSelectableNode(selNode, selOffset, aAction,
>+                              address_of(nearNode));

And it's referred at here as a pointer to nsCOMPtr<nsINode>.

> nsresult
>-HTMLEditRules::FindNearSelectableNode(nsIDOMNode* aSelNode,
>+HTMLEditRules::FindNearSelectableNode(nsINode* aSelNode,
>                                       int32_t aSelOffset,
>                                       nsIEditor::EDirection& aDirection,
>-                                      nsCOMPtr<nsIDOMNode>* outSelectableNode)
>+                                      nsCOMPtr<nsINode>* outSelectableNode)

However, you specify nsCOMPtr<nsINode>* here instead of nsCOMPtr<nsIDOMNode>*.

> {
>   NS_ENSURE_TRUE(aSelNode && outSelectableNode, NS_ERROR_NULL_POINTER);
>   *outSelectableNode = nullptr;
> 
>-  nsCOMPtr<nsIDOMNode> nearNode, curNode;
>+  nsCOMPtr<nsIContent> nearNode, curNode;
>   if (aDirection == nsIEditor::ePrevious) {
>     NS_ENSURE_STATE(mHTMLEditor);
>-    nsresult rv =
>-      mHTMLEditor->GetPriorHTMLNode(aSelNode, aSelOffset, address_of(nearNode));
>-    if (NS_WARN_IF(NS_FAILED(rv))) {
>-      return rv;
>+    nearNode = mHTMLEditor->GetPriorHTMLNode(aSelNode, aSelOffset);
>+    if (NS_WARN_IF(!nearNode)) {
>+      return NS_ERROR_FAILURE;
>     }
>   } else {
>     NS_ENSURE_STATE(mHTMLEditor);
>-    nsresult rv =
>-      mHTMLEditor->GetNextHTMLNode(aSelNode, aSelOffset, address_of(nearNode));
>-    if (NS_WARN_IF(NS_FAILED(rv))) {
>-      return rv;
>+    nearNode = mHTMLEditor->GetNextHTMLNode(aSelNode, aSelOffset);
>+    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;
>     }
> 
>+    nsCOMPtr<nsIContent> node;
>     if (aDirection == nsIEditor::ePrevious) {
>       NS_ENSURE_STATE(mHTMLEditor);
>-      nsresult rv = mHTMLEditor->GetPriorHTMLNode(aSelNode, aSelOffset,
>-                                                  address_of(nearNode));
>-      if (NS_WARN_IF(NS_FAILED(rv))) {
>-        return rv;
>+      node = mHTMLEditor->GetPriorHTMLNode(aSelNode, aSelOffset);
>+      if (NS_WARN_IF(!node)) {
>+        return NS_ERROR_FAILURE;
>       }
>     } else {
>       NS_ENSURE_STATE(mHTMLEditor);
>-      nsresult rv = mHTMLEditor->GetNextHTMLNode(aSelNode, aSelOffset,
>-                                                 address_of(nearNode));
>-      if (NS_WARN_IF(NS_FAILED(rv))) {
>-        return rv;
>+      node = mHTMLEditor->GetPriorHTMLNode(aSelNode, aSelOffset);
>+      if (NS_WARN_IF(!node)) {
>+        return NS_ERROR_FAILURE;
>       }
>     }
>+    nearNode = Move(node);

Then, |node| is nsCOMPtr<nsIContent>,

>   }
> 
>   // scan in the right direction until we find an eligible text node,
>   // but don't cross any breaks, images, or table elements.
>   while (nearNode && !(EditorBase::IsTextNode(nearNode) ||
>                        TextEditUtils::IsBreak(nearNode) ||
>                        HTMLEditUtils::IsImage(nearNode))) {
>     curNode = nearNode;
>+    nsCOMPtr<nsIContent> node;
>     if (aDirection == nsIEditor::ePrevious) {
>       NS_ENSURE_STATE(mHTMLEditor);
>-      nsresult rv =
>-        mHTMLEditor->GetPriorHTMLNode(curNode, address_of(nearNode));
>-      if (NS_WARN_IF(NS_FAILED(rv))) {
>-        return rv;
>+      node = mHTMLEditor->GetPriorHTMLNode(curNode, aSelOffset);
>+      if (NS_WARN_IF(!node)) {
>+        return NS_ERROR_FAILURE;
>       }
>     } else {
>       NS_ENSURE_STATE(mHTMLEditor);
>-      nsresult rv = mHTMLEditor->GetNextHTMLNode(curNode, address_of(nearNode));
>-      if (NS_WARN_IF(NS_FAILED(rv))) {
>-        return rv;
>+      node = mHTMLEditor->GetNextHTMLNode(curNode, aSelOffset);
>+      if (NS_WARN_IF(!node)) {
>+        return NS_ERROR_FAILURE;
>       }
>     }
>     NS_ENSURE_STATE(mHTMLEditor);
>+    nearNode = Move(node);

And also here.

>   }
> 
>   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 = do_QueryInterface(nearNode);
>+    *outSelectableNode = nearNode;

And nearNode is set to the new out argument. So, the argument can be nsCOMPtr<nsIContent>.  So, I think that you don't need to do the first change.

Comment 4

[(no longer active)]

Attachment: Part 1: Rewrite HTMLEditRules::FindNearSelectableNode() to use internal DOM APIs

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36d5f919c6b2
Part 1: Rewrite HTMLEditRules::FindNearSelectableNode() to use internal DOM APIs; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/07f89c0d5e17
Part 2: Remove some dead code; r=masayuki

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/36d5f919c6b2
https://hg.mozilla.org/mozilla-central/rev/07f89c0d5e17