Created attachment 70565 [details]
Test case to reproduce the problem - runs on both GNOME 2.14 and GNOME 2.15.90

Run this test case in an xterm using "python bug_350623.py".  Give a gnome-terminal focus and type in it.

Good output from this test case on gnome-terminal-2.14.2 looks like this:

Text line at caret offset 981 starts at 981 and ends at 982

Bad output from this test case on gnome-terminal-2.15.4 looks like this:

Text line at caret offset 1944 starts at -1081530984 and ends at -1211735792

In digging into the code, I see all the getTextAtOffset stuff is handled in vte, so I'm reassigning this to vte, even if I have no idea what is causing the bug yet.

Created attachment 70596 [details] [review]
Bogus patch that fixes the symptom, but not really the bug

I did some digging around and I think I see where the problem is occurring.  In vetaccess.c:vte_terminal_accessible_update_private_data_if_needed, there's some logic that is determining where the caret is.  It seems as though this always thinking the caret position is at the end of the text buffer (i.e., caret == snapshot_characters->len).  This then results in vte_terminal_accessible_get_text_somewhere crapping out, setting the text to "" and leaving the return values for start_offset and end_offset with their uninitialized values.

I then did some more debugging and it looks like the return value is not so hot for the following call:

			attrs = g_array_index(priv->snapshot_attributes,
					      struct _VteCharAttributes,
					      offset);

For example, I can see attrs.row==0 and attrs.column==0 for multiple offsets in the same snapshot.  This doesn't seem right to me.  As a result, I think this is throwing the caret calculation logic off and we'll always end up with the caret==len.

Soo...I hacked very badly (it took me several hours just to get to this point in my analysis of this bug, so I'm rather tired right now) and came up with the attached junk I hope nobody checks in unless it's a last resort.  I even left the debug "printf"'s in there as a warning to not take this patch unless you really have to.  But, hopefully, someone more experienced and knowledgeable with vte will have some insight into what broke.  My suspicion is that it might somewhere where VteCharAttributes is calculated in vte.c.

	/* Get the offsets to the beginnings of each line. */
	caret = -1;
	for (i = 0; i < priv->snapshot_characters->len; i++) {
		/* Get the attributes for the current cell. */
		offset = g_array_index(priv->snapshot_characters,
				       int, i);
		attrs = g_array_index(priv->snapshot_attributes,
				      struct _VteCharAttributes,
				      offset);
		/* If this cell is "before" the cursor, move the
		 * caret to be "here". */
		printf("LOOP: i=%d caret=%d crow=%d ccol=%d offset=%d row=%d col=%d\n", i, (int) caret, (int) crow, (int) ccol, (int) offset, (int) attrs.row, (int) attrs.column);
		if (attrs.row < crow) {
		        caret = i + 1;
		} else if (attrs.row == crow) {
		        if (attrs.column < ccol) {
		                caret = i + 1;
		        } else {
		                break;
		        }
		} else {
		        break;
		}
	}

	printf("CARET: %d\n", caret);

I should add that the AWFUL DON'T USE THIS PATCH hack I gave only fixed part of the problem.  Something is still terribly wrong with getTextAtOffset when I pass it other offsets - I don't seem to get the correct rectangular regions for where the text is drawn on the screen and such.  I'm hoping this is all related to one small breakage somewhere.  If you want a test case for that, I can spin one up.  Alternatively, you can run Orca and try to navigate gnome-terminal window in flat review mode.  Orca will draw a black rectangle around the text associated with the following keystrokes:

KP_{7,8,9}: {previous,current,next} line
KP_{4,5,6}: {previous,current,next} word
KP_{1,2,3}: {previous,current,next} character

Please note that this regression renders gnome-terminal almost completely useless with respect to accessibility.  :-(  I really really hope this can be fixed for GNOME 2.16.

For further information: I began the process of building/testing one release at a time and found that the breakage occurred when going from vte-0.13.2 to vte-0.13.3.  I'll keep digging, but if anyone more knowledgeable has some insight, please share.  :-)

I'm going to guess that something in this change introduced something that might have required some changes to vteaccess.c:

2006-07-10  Behdad Esfahbod  <behdad@gnome.org>

        Bug 121904 â copy-paste of empty line
        Bug 25290 â Small UI tweak to select-by-word (only select only letter
        at a time for non-word characters)
        Bug 339986 â Patch to select localized strings exactly

        * src/vte-private.h:
        * src/vte.c (_vte_terminal_set_default_attributes),
        (_vte_terminal_insert_char), (vte_terminal_is_word_char),
        (vte_same_class), (vte_terminal_get_text_range_maybe_wrapped),
        (vte_terminal_extend_selection):
        * src/vteseq.c (vte_sequence_handler_screen_alignment_test):
        Fix a bunch of selection-related issues.  We now recognize explicitly
        put space at the end of lines, and copy/paste it.  The other change is
        that non-wordchar characters are not grouped together.  Also fixes the
        fallback on Unicode properties that I added two commits ago.

I'll keep digging, but any help/insight from the maintainers would be appreciated!  :-)

I really have no idea.  And seems like my vte/g-t is not built with a11y enabled, so the test program doesn't output anything here.

(In reply to comment #7)
> I really have no idea.  And seems like my vte/g-t is not built with a11y
> enabled, so the test program doesn't output anything here.

I think you may need to enable a11y in your desktop environment.  You can do so via the System -> Preferences -> Assistive Technology preferences menu (or just run gnome-at-properties).  Then, you need to log out and back in again.  If you do this, vte/g-t should pick everything up automagically without needing to be built in any special way. BTW, just running /usr/bin/vte with the test app should be sufficient to demonstrate the problem. I'm still trying to wrap my mind around just what is going on inside vte and vteacess, but I've got no insight yet.

More information: I think that I've isolated the changes down to one file.  The break seemed to occur with version 1.471 of vte.c (that is, 1.470 works and 1.471 doesn't).  Here's the ChangeLog entry 

2006-07-10  Behdad Esfahbod  <behdad@gnome.org>

        Bug 121904 – copy-paste of empty line
        Bug 25290 – Small UI tweak to select-by-word (only select only letter
        at a time for non-word characters)
        Bug 339986 – Patch to select localized strings exactly

        * src/vte-private.h:
        * src/vte.c (_vte_terminal_set_default_attributes),
        (_vte_terminal_insert_char), (vte_terminal_is_word_char),
        (vte_same_class), (vte_terminal_get_text_range_maybe_wrapped),
        (vte_terminal_extend_selection):
        * src/vteseq.c (vte_sequence_handler_screen_alignment_test):
        Fix a bunch of selection-related issues.  We now recognize explicitly
        put space at the end of lines, and copy/paste it.  The other change is
        that non-wordchar characters are not grouped together.  Also fixes the
        fallback on Unicode properties that I added two commits ago.

Unfortunately, this was a rather substantial change (+81 -118 lines) and I don't understand the implications of everything that was done, with the exception that it appears to have broken accessibility ;-).  I'll keep digging.

I reviewed commit again, nothing suspicious.  Any way to reproduce the bug without having to logout and login?

(In reply to comment #10)
> I reviewed commit again, nothing suspicious.  Any way to reproduce the bug
> without having to logout and login?

You only need to enable accessibility for your session once.  Once you've done this, it is sticky and is enabled from session to session.  The reason you need to log out the first (and only) time you enable accessibility is so that the appropriate system services (e.g., at-spi-registryd) and libraries will be loaded/used by various apps, including vte.

The test case listed as an attachment to this bug report is able to demonstrate the problem, but I think it is only running into the effects of this change much further down the stream.

If you want to debug in vte itself, I've noticed something very odd happening with the attributes.  I added the following lines to vteaccess.c:vte_terminal_accessible_update_private_data_if_needed:

		/* Find offsets for the beginning of lines. */
		for (i = 0, row = 0; i < priv->snapshot_characters->len; i++) {
			/* Get the attributes for the current cell. */
			offset = g_array_index(priv->snapshot_characters,
					       int, i);
			attrs = g_array_index(priv->snapshot_attributes,
					      struct _VteCharAttributes,
					      offset);
-->			printf("LOOP: i=%d offset=%d row=%d col=%d\n",
-->			       i, offset, attrs.row, attrs.column);

When I look at the resulting output, I see some odd lines appearing when vte.c:1.471 is used versus when vte.c:1.470 is used:

LOOP: i=30 offset=30 row=0 col=30
LOOP: i=31 offset=31 row=0 col=0     <----- this looks wrong
LOOP: i=32 offset=32 row=1 col=0

I think the attributes might be being calculated incorrectly?

Created attachment 70948 [details]
vte.c from 0.13.3, but with 0.13.2 vte_terminal_get_text_range_maybe_wrapped

This file demonstrates that the breakage appears to be isolated to the changes made to the vte_terminal_get_text_range_maybe_wrapped method of vte.c between 0.13.2 and 0.13.3.  There's a #if at line 4728 - if you set this to 1, you get the 0.13.2 method, which doesn't break accessibility.  If you get this to 0, you get the 0.13.3 method, which breaks accessibility.  

There were substantial changes made to this method between 0.13.2 and 0.13.3.  I've not taken the time to become a vte developer yet, so I don't understand what's going on in this code.  Please see the other comments in this bug report, however, for information on what appears to be going wrong (i.e., row and col of attributes seem to be incorrectly set to 0 for some offsets).  Hopefully the maintainers will be able to fix this regression for the upcoming tarball release.

If it helps the maintainers any to fix this regression, I was able to verify that using the 0.13.2 vte_terminal_get_text_range_maybe_wrapped method of vte.c makes the problem go away in GNOME CVS HEAD. 

Created attachment 71014 [details] [review]
Patch to eliminate the resize of the array before calling vte_g_array_fill

I think I found the cause of the regression: there was a call to g_array_set_size(...string->len...) just prior to a call to vte_g_array_fill(...string->len...).  The logic of vte_g_array_fill is such that it will keep appending values to the array until the array's length matches the desired length.  Because the array was resized to string->len just prior to the call, the array is already at the desired length when vte_g_array_fill is called, thus resulting in a no-op.

Furthermore, vteaccess.c creates the attributes array with a call to g_array_new, setting the 'clear' parameter to TRUE.  This has the effect of making sure any call to resize will zero out any new elements.  

Combining these two, we end up with attribute entries with all 0's in them.

This patch merely removes the call to g_array_set_size.  With this, accessibility works again.

I'm bumping this up to urgent because "this bug blocks usability of a large portion of the product, and really should be fixed before the next planned release."  Please review/submit/release this patch for the next round of GNOME 2.16 tarballs (this coming Monday, August 21).

Thanks.

2006-08-16  Willie Walker <william.walker@sun.com>

        Bug 350623 – Accessible text getTextAtOffset is broken

        * src/vte.c (vte_terminal_get_text_range_maybe_wrapped): Do not
        resize array before calling vte_g_array_fill since it
        nullifies any effect vte_g_array_fill will have.

I've had a report from a user that this is broken again with the latest code.  :-(  I'm going to re-open this as a means to expose this issue while I also update/test with the latest stuff.

After finally being able to get vte from GNOME CVS HEAD to run without SEGV'ing (see http://bugzilla.gnome.org/show_bug.cgi?id=353399), I was able to verify that the code from GNOME CVS HEAD no longer has this regression, so Behdad must've fixed it.  :-)  Thanks!