Crash when typing after pressing Enter in text-box

[yagodlt]

Inkscape version: 0.48+dev+1250
OS version: Linux Mint 13 64bit / Gnome Shell

Expected behaviour:
Here I am, typing away happily in a text box when, at some point, decide to change line. So I press Enter (return... whatever) and keep typing happily in a new line.

Actual behaviour:
When I try to keep typing after pressing Enter, Inkscape crashes

Steps to reproduce (in case they aren't obvious enough):

* open inkscape
* use the text tool (F8 or 't') and click and drag to create a text box
* type **happily** a couple of lines (or more/less, it's up to you) inside that text box.
* press Enter
* keep typing

At this point, Inkscape should crash.

[su_v (suv-lp)]

Reproduced with Inkscape 0.48+devel r12506 on OS X 10.7.5.

The crash seems not consistently reproducible: adding hard line breaks in flowed text is definitely broken, sometimes producing a crash, sometimes random strings (in one instances the strings appeared to be taken from Inkscape's user interface) get inserted when pressing 'Enter'.

Testing with archived builds:
- not reproduced with r12487 and earlier revisions
- reproduced with r12488 and later revisions

Likely introduced with the merge of the emf-support branch in r12488:
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/12488>

[su_v (suv-lp)]

Attaching backtrace (crash with Inkscape 0.48+devel r12596, OS X 10.7.5, GTK+/X11 2.24, glib 2.36.4)

Crash also reproduced with r12504 (local build) on Ubuntu 12.10 (VM 64bit)

[su_v (suv-lp)]

Correction (revision number):
- (crash with Inkscape 0.48+devel r12596, OS X 10.7.5, GTK+/X11 2.24, glib 2.36.4)
+ (crash with Inkscape 0.48+devel r12506, OS X 10.7.5, GTK+/X11 2.24, glib 2.36.4)

[su_v (suv-lp)]

@David - any chance you could take a closer look?

Based on a few quick tests with archived builds of the original emf branch, this regression seems to have been introduced with the changes in
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/emf-support/revision/11739>

[su_v (suv-lp)]

Earlier reports likely triggered by the same underlying issue:
- Bug #1221025 “Inkscape crashes when import text from buffer via Ctrl+V”
  <https://bugs.launchpad.net/inkscape/+bug/1221025>
- Bug #1222410 “Crash when modifying pasted text in a text box”
  <https://bugs.launchpad.net/inkscape/+bug/1222410>

[David Mathog (mathog)]

Interesting. It does that only if the text box is dragged out to give a particular size, then text is entered. If instead one does ^t then just start typing there is no problem with the enter. Nor does there seem to be a problem if one edits an existing text box, or at least I have not yet crashed doing so.

Will look into it.

[Johan Engelen (johanengelen)]

I can't reproduce this with trunk rev12506 on Windows.

[yagodlt (yagodlt)]

@mathog - I can confirm that I can reproduce it when editing an existing box on 0.48+dev r1250.
I'm not trying to teach my grandmother to suck eggs, but have you tried to continue typing after breaking the line? 'cause that's when it crashes for me...

[David Mathog (mathog)]

Sorry, this is long, as I could not find the exact problem, just narrow it down a bit.

1: It only happens with flowed text.
2: it only happens on the wrapped part of a continued line.
3: It only happens on a continued line when the cursor is not at the end of the string.

To demonstrate this:

1. start inkscape
2. drag out a small text box, wide enough for just a few letters and tall enough for several lines.
3. type "abc def" such that it wraps and "def" is on the second line.

When the first line is being typed the cursor moves and stays at the end (right side) of the line.
When it wraps and"def" is entered, the cursor stays at the start (left side) of the line.

If an "enter" key is pressed with the cursor in that position, the program will crash when the next regular letter is pressed.

However, if the keyboard arrow keys are used to move the cursor to the end of the line, then "enter" is nontoxic.

Inserting an "enter" anywhere but after the last character on a wrapped line is toxic.

Normal text has none of these issues.

So I think one key part of the problem is that there is a bug in the cursor positioning for wrapped lines in flowed text. The crash seems to result from that.

The program crashes in text-editing.cpp at insert_into_spstring() at the string->replace because when "\n" is inserted in one of the toxic scenarios the string's length gets out of sync with the iterator, with the iterator being one position past the end of the string. The \n character is not entered in the flowed text scenario, or only entered part way. The iter_at value passed into this function has a value as if the \n is there. However, iterate from string->begin to string->end and it comes up one short - no \n has been inserted. Conclusion, somewhere the iterator is incremented without the character actually being inserted. Seems like the line should have split. In any case, the iterator should never be past the length of the line.

In all cases where a '\n' is entered and the execution passes through the part of sp_te_insert_line() under
the conditional

    } else if (SP_IS_STRING(split_obj)) {

then it will crash on the next normal letter entered. So I think the block of code from about lines 422 -440 in text-editing.cpp is broken, but I do not see how.

The following is probably a related problem:

Draw out a tall narrow text box.
Enter "abc def" such that it wraps.
Use arrow keys to move cursor to right of "f".
Press "enter"
Enter "ghi jkl" such that it wraps.
Use mouse or arrow keys to place cursor between "d" and "e".
Press "enter".

Line should split, instead a blank line is added between "def" and "ghi".
Cursor between "a" and "b" or "b" and "c" splits as it should.

So the first part of wrapped line edits properly, subsequent parts do not.

[yagodlt (yagodlt)]

Very interesting and thorough analysis, thanks! At least now I know what to expect and how to go around it while the bug gets fixed :D

[yagodlt (yagodlt)]

Very likely related:

Inkscape crashes when trying to modify some text within a text box if you select it with click+drag and then try to type something to replace it.

Here "enter" is not involved, though...

[yagodlt (yagodlt)]

I'm sorry to say that the arrow keys workaround does not work for me. It still crashes.

[David Mathog (mathog)]

Tracked the problem down to revision 11739 of branch lp988601. Ugh. That patch was a beast - it implemented text decorations plus various fixes for R->L languages. Possibly some change in Layout-TNG-Compute.cpp that feeds back badly with code elsewhere that implements flowed text (but is harmless for normal text).

[David Mathog (mathog)]

I'm going around in circles on this one. Maybe new eyes can spot the issue.

In tests setting up a r11738 revision and then changing everything except Layout-TNG-Computer.cpp to r11739 produced a version that I could not get to crash, and where the cursor moved appropriately in flowed text. So the problem seems to be at least primarily in Layout-TNG-Compute.cpp, and specifically, in _outputLine() in that file. (Reverting the rest of it to r11739 did not introduce the bug.)

The modifications I made to outputLine() to handle Hebrew (and other languages') diacritical marks is apparently incompatible with the flowed text implementation. The current Trunk version of the code in this file dates from r11739 of branch lp988601. It does normal text correctly, and it handles the attached diacritical mark example correctly. r11738 of branch lp988601 does normal text and flowed text correctly, but it does the wrong thing with the diacritical marks, advancing on each of those glyphs when it should not be doing so. If somebody can figure out how to fold that in without breaking flowed text, more power to you!

The attached example is Hebrew with vowels (diacritical marks). The rightmost character, that looks like a sideways 3 should have a dot in its upper right corner and a mark beneath it. It looks more or less like it should if viewed in Firefox, except the exclamation mark is on the wrong side.

[David Mathog (mathog)]

Note all revision numbers refer to the lp988601 branch.

[su_v (suv-lp)]

Daviid Mathog wrote:
> (…) Maybe new eyes can spot the issue.
and
> Note all revision numbers refer to the lp988601 branch.

Comment #4 has the link to the specific revision which introduced the regression, repasting in case someone is willing to help out here:
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/emf-support/revision/11739>

[David Mathog (mathog)]

This patch might be sufficient. Please somebody give it a try, beat on it some in flowed text editing, and then if it seems OK it can go into trunk.

[su_v (suv-lp)]

Testing patch 'tng2.patch' applied to Inkscape 0.48+devel r12522 on OS X 10.7.5 (64bit) with the known 'steps to reproduce' [*]:

AFAICT the patch works as expected - no more crashes when
a) adding text after a line break (new paragraph) in flowed text (this bug #1224486),
b) pasting text with newlines from the clipboard into an active flowed text object (bug #1221025, bug #1222410).

--
[*] All tests have been done with (fresh) default preferences, using new flowed text objects created in a new document based on the default template. Locale is 'en_US.UTF-8', font is default generic 'sans-serif' (resolves to 'DejaVu Sans' with my current font configuration).

[Martin Owens (doctormo)]

Tested here, it also works and no more text crashes. r12522 Linux 32bit.

One note, you should change that from unsigned to int, I had a look and the cairo code that the num_glyphs is used in require signed ints. So get rid of that compile time warning.

[jazzynico (jazzynico)]

Patch tested successfully on Windows XP, Inkscape trunk revision 12526.
Also fixes bug #1221025 and bug #1222410.

[Martin Owens (doctormo)]

That's three tests, I've merged the patch into trunk see r12529. Thanks David!

[su_v (suv-lp)]

Bug #1221025 and bug #1222410 linked as duplicates to this one (same bug, differently triggered).

Closing as 'Fix released'.

[yagodlt (yagodlt)]

Sorry guys, I'm not a developer and I have no clue what to do with that patch. I've looked everywhere for the file 'src/libnrtype/Layout-TNG-Compute.cpp' (I thought I'd overwrite it) but couldn't find it... Anyone care to shed some light?

Much appreciated

[su_v (suv-lp)]

yagolf - how to you install inkscape trunk on your system?

The patch has been committed to trunk (in revision 12519) - i.e. you now do not have to deal with that patch yourself at all:
- If you install from the official inkscape trunk PPA, you just wait for the next available update (>= r12529 will include the fix),
- if you compile inkscape trunk yourself, you just pull the latest changes from bzr, and recompile.

[yagodlt (yagodlt)]

Thanks. Will wait for the next available update then.