641466 - surrogate handling is broken when deleting Plane-1 unicode in <textarea>

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Jonathan Kew [:jfkthame]]

The testcase (see URL) has an <input> element and a <textarea>, both containing 4 Shavian letters. Clicking at the end of the text in the <input> element and backspacing 4 times correctly deletes the letters, one by one. In the <textarea>, however, the first 3 backspaces work correctly, but the last one leaves an unpaired high surrogate in the textarea, and an extra backspace keystroke is needed to completely delete it.

Comment 1

[Boris Zbarsky [:bzbarsky]]

That's really weird, since the code should be the same...

Comment 2

[Jonathan Kew [:jfkthame]]

(In reply to comment #1)
> That's really weird, since the code should be the same...

Well, in both cases the deletion is handled through nsPlaintextEditor::DeleteSelection(). But there's an important difference, in that the <input> element's text is held in a single node of type TEXT_NODE, whereas in the <textarea>, each codepoint seems to be contained in an individual ELEMENT_NODE. This means that the surrogate check at http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/text/nsPlaintextEditor.cpp#712 applies in the <input> case, but does NOT apply to the corresponding backspace in <textarea>.

There must be something else that detects and handles the surrogates in the <textarea> case, but it fails when the surrogate pair is the only remaining content in the editor.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> each codepoint seems to be contained in an individual ELEMENT_NODE

Er.... that shouldn't be happening!

When we hit that code, is |node| the textarea itself or something?

Comment 4

[Jonathan Kew [:jfkthame]]

(In reply to comment #3)
> > each codepoint seems to be contained in an individual ELEMENT_NODE
> 
> Er.... that shouldn't be happening!

And it isn't, sorry. I was misled by the behavior of nsTextEditor::GetStartNodeAndOffset when called from http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/text/nsPlaintextEditor.cpp#700, where it always seems to return an offset of 1 when the insertion point is at the end of a <textarea>, regardless of the amount of text present. I guess that's because it is returning the textarea node, with offset=1 meaning the selection is _beyond_ the TEXT_NODE (its first child?) that actually contains the text. Or something like that.

If the insertion point is somewhere _within_ the text, GetStartNodeAndOffset returns an offset that reflects the number of characters before the insertion point, as expected. It's only when it is at the _end_ of the text that it returns offset=1 (and a different node).

Comment 5

[(no longer active)]

For the three first back-spaces, this is the code which extends the selection to the surrogate range:  <http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/base/nsEditor.cpp#4759>.

I think this is a bug in checking the index bounds check there.

Comment 6

[(no longer active)]

That is, my mistake in bug 558983.

Comment 7

[(no longer active)]

Attachment: Patch (v1)

Thanks Jonathan for finding this issue!

Comment 8

[Jonathan Kew [:jfkthame]]

(In reply to comment #7)
> Created attachment 519290 [details] [diff] [review]
> Patch (v1)
> 
> Thanks Jonathan for finding this issue!

To give credit where it's due, this was originally mentioned by [e-mail] in bug 640673 comment #1, but I filed it separately as it's a separate issue from the original text-input problem described in that bug.

Comment 9

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/ffc9ebd68b40