Bug 3654 – cursor position accessors on KWQTextArea require \n

• Component: Forms
• Status: VERIFIED FIXED

[Kevin Ballard]

The accessors for the cursor position on KWQTextArea are not line ending-agnostic - they require \n (and 
work with \r\n simply because you can't put the cursor between the \r and the \n). This is actually only 
used to save the cursor position when changing the text value of the field, so the only way to test it is to 
give the field a text value using carriage returns, set the selection, then change the text value to 
something else with newlines, and check the selection. The cursor will be stuck in the first line of the field 
instead of where it should be.

[Kevin Ballard]

The reason this depends on 3401 is simply for the testcase - it requires the ability to get/set the 
selection.

[Kevin Ballard]

Created attachment 2554 [details]
A testcase for the bug

[Kevin Ballard]

I have a fix for this already, but I can't create a proper patch until bug 3401 is committed, because I have a 
lot of changes in the same file for bug 3401 which cause problems with generating a patch just for this 
file.

[Kevin Ballard]

Created attachment 2614 [details]
A patch for this problem

[Darin Adler]

I don't understand how the test case works. I thought that you changed the setter so that it would convert 
non-standard line endings to standard ones. If so, then this doesn't test the code in KWQTextArea, does 
it?

[Kevin Ballard]

No, I stopped doing that when I added the updateFromElement() call - instead I just invalidated the 
cached value, so the next retrieve would get the translated value, but the textarea still has the newline 
style of the input text.

What this does is it sets the textarea text to be all carriage returns, so the textarea's cursor functions 
thinks it's a single paragraph (pre-patch). It then sets the textarea text to be all newlines (plus the 
exclamation mark - otherwise it will think it's the same text and not bother calling 
updateFromElement), which calls updateFromElement. That fetches the cursor position, gets a large 
index in paragraph 0 (assuming the selection is past a carriage return), sets the text to use newlines, 
and then tries setting the cursor position again. The cursor ends up being in the last position in 
paragraph 0, which in the testcase means it will be in index 7 instead of index 10 where it belongs.

The fix makes the getCursorPosition... actually return the right paragraph/index, so when setting it 
again it works.

And I just realized that this doesn't actually test the line ending-agnostic behaviour of 
setCursorPosition..., just getCursorPosition.... Let me see if I can get a testcase working for 
setCursorPosition... as well.

[Kevin Ballard]

Created attachment 2631 [details]
A better testcase

Ok, this testcase tests both getCursorPosition... and setCursorPosition... (and
hence, RangeOfParagraph() as well).

[Darin Adler]

Comment on attachment 2614 [details]
A patch for this problem

Looks good, r=me.