722406 - Cleanup nsHTMLEditor::SetCaretInTableCell

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part a: Introduce nsISelection::CollapseNative

Editor often calls nsISelection::Collapse, and increasingly has nsINodes to pass, so it makes sense to allow passing an nsINode directly.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part b: Cleanup nsHTMLEditor::SetCaretInTableCell

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 592794 [details] [diff] [review]
Part a: Introduce nsISelection::CollapseNative

update uuid

Comment 3

[(no longer active)]

Comment on attachment 592795 [details] [diff] [review]
Part b: Cleanup nsHTMLEditor::SetCaretInTableCell

Review of attachment 592795 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/libeditor/html/nsHTMLEditor.cpp
@@ +4038,5 @@
> +  if (!content || !content->IsHTML()) {
> +    return false;
> +  }
> +
> +  // REVIEW NOTE: this didn't handle th before

Noted.  Please remove this comment.  :-)

@@ +4043,5 @@
> +  if (!nsHTMLEditUtils::IsTableElement(content->AsElement())) {
> +    return false;
> +  }
> +
> +  // Find deepest child

This is actually a lie, isn't it?  ;-)  I'd remove this comment, since it's pretty evident what the loop is doing.

@@ +4049,5 @@
> +  while (node->HasChildren()) {
> +    node = node->GetFirstChild();
> +  }
> +
> +  // Set selection at beginning of deepest node

s/deepest/the found/

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/657cac6e1019
https://hg.mozilla.org/mozilla-central/rev/0f7aee23bc51