[Bug 5968] Richedit crashes when adding a chr(10) or chr(13)

http://bugs.winehq.org/show_bug.cgi?id=5968

--- Comment #2 from Alex Villacís Lasso   2007-10-20 20:19:01 ---
Created an attachment (id=8693)
 --> (http://bugs.winehq.org/attachment.cgi?id=8693)
Proof of concept on possible way to handle \r\r\n sequence for 1.0 emulation

I have taken interest in this particular bug. From digging into the code, I 
have found out the following information, which will be useful in order to
finally eliminate this bug.

For a start, the test application is trying to use a rich edit control in 1.0
emulation mode. This is important, because the bug concerns the behavior of
paragraph breaks when inserting non-standard sequences. To recap, the 1.0
emulation involves (but is not necessarily limited to) treating \r\n pairs as
paragraph breaks, and returning, in turn, paragraph breaks as \r\n pairs. In
contrast, 2.0 and higher treat \r, \n, and \r\n as valid indicators for 
paragraph breaks, but treat them all as a single character, and are all 
represented as \r. There are some places in builtin riched20 that make 
allowances for this, but due to the lack of richedit 1.0 tests, they are not
currently exercised.

When pressing the button labeled "Crash", the app code tries to insert the 
(non-standard) sequence \r\r\n . For riched20 in 2.0 and higher, this should 
insert a single space. For 1.0, this should insert the characters as they are,
and advance the cursors by a delta of 3 (from the point of view of the app).
The bug is that currently, builtin treats this sequence as *two* paragraph
breaks (\r, then \r\n), and then the emulation code shows a cursor delta of 4
(twice the \r\n sequence). The app expected a delta of 3, so it panics and
throws an exception.

The sequence that leads to the exception goes somewhat like this:

1) Control starts with contents of "RichEdit1\r\n", which should mean, 9 
   printable characters, plus one paragraph break composed of two characters,
   for a total of 11 characters. EM_EXGETSEL agrees with this and returns a
   CHARRANGE of (11,11).
2) User presses "Crash" button.
3) A EM_REPLACESEL is sent to the control, with the sequence "\r\r\n" (3
bytes).
4) Current builtin takes this to mean "\r is one paragraph break, and \r\n is
   another, so two paragraph breaks should be inserted".
5) The Delphi app disagrees: "\r is a standalone character, but non-printable,
   and only \r\n counts as a paragraph break. Only one paragraph break is
   inserted, but all three characters should end up in the control".
6) App then sends a EM_EXGETSEL message to query the current selection.
7) Due to the reasoning at 4), builtin inserted two paragraph breaks. However,
   since the app requested a 1.0 control, builtin converts each paragraph break
   into \r\n pairs. For an initial selection at (11,11), builtin reports now
   (15,15).
8) Due to the reasoning at 5), the app expects to see (14,14). When builtin
   reports (15,15), the app screams and throws an exception.

The attached patch is a proof of concept. It changes the behavior of text
insertion, so that \r by itself won't create a new paragraph - only \r\n
will. An unpaired \r will be inserted as a normal character. This patch is 
enough to prevent the app from throwing an exception. However, this patch makes
one extra space (the solitary \r) appear as selectable. So this is not a proper
solution.

If this patch is taken as a basis for a complete solution, it will need 
modifications to the painting routines in order to refuse to select solitary
trailing \r characters at the end of paragraphs. Most importantly, this will
need a fully-fledged riched32 1.0 test suite.