1406215 - Remove the usage of nsINode::GetChildAt() from nsTextServicesDocument::GetCollapsedSelection()

Status: RESOLVED FIXED

(Core :: DOM: Editor, enhancement)

Comment 1

[(no longer active)]

Attachment: Part 1: Remove a useless bounds check for a selection range offset

This offset is obtained from a Range, so it doesn't need to be
validated.

Comment 2

[(no longer active)]

Attachment: Part 2: Remove the usage of nsINode::GetChildAt() from nsTextServicesDocument::GetCollapsedSelection()

Comment 3

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

Comment on attachment 8915783 [details] [diff] [review]
Part 2: Remove the usage of nsINode::GetChildAt() from nsTextServicesDocument::GetCollapsedSelection()

>@@ -2384,23 +2384,20 @@ nsTextServicesDocument::GetCollapsedSelection(nsITextServicesDocument::TSDBlockS
>   nsIContent* saveNode;
>   if (parent->HasChildren()) {
>     // XXX: We need to make sure that all of parent's
>     //      children are in the text block.
> 
>     // If the parent has children, position the iterator
>     // on the child that is to the left of the offset.
> 
>-    uint32_t childIndex = offset;
>-
>-    if (childIndex > 0) {
>-      childIndex -= 1;

Current implementation does not try to retrieve previous child if the offset is 0.

>+    nsIContent* content = range->GetChildAtStartOffset();
>+    if (content) {
>+      content = content->GetPreviousSibling();
>     }

So, looks like that you need to check:

if (content && parent->GetFirstChild() != content) {
  content = content->GetPreviousSibling();
}

If I misunderstood, ignore this review. Otherwise, fix it before landing.

Comment 4

[(no longer active)]

Good catch.  You're right!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/756a3fa73513
Part 1: Remove a useless bounds check for a selection range offset; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/3edd962139b6
Part 2: Remove the usage of nsINode::GetChildAt() from nsTextServicesDocument::GetCollapsedSelection(); r=masayuki

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/756a3fa73513
https://hg.mozilla.org/mozilla-central/rev/3edd962139b6