After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 166637 - Accessible's caret position sometimes wrong
Accessible's caret position sometimes wrong
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
0.11.x
Other All
: Normal normal
: ---
Assigned To: VTE Maintainers
VTE Maintainers
AP2
Depends on:
Blocks:
 
 
Reported: 2005-02-08 07:36 UTC by Samuel Thibault
Modified: 2015-02-01 19:43 UTC
See Also:
GNOME target: ---
GNOME version: 2.7/2.8


Attachments
not-garbaged patch which enhances the first bug example (719 bytes, patch)
2005-05-17 21:08 UTC, Samuel Thibault
committed Details | Review
Proposed patch. (2.56 KB, patch)
2013-01-02 04:58 UTC, Jason White
rejected Details | Review
Pyatspi script to watch for caret-moved events. (203 bytes, text/x-python)
2015-01-22 00:56 UTC, Mike Gorse
  Details

Description Samuel Thibault 2005-02-08 07:36:23 UTC
Please describe the problem:
Hi,

The Accessible caret's position is sometimes wrong (out of date actually maybe ?)


Steps to reproduce:
- run gnome-terminal,
- run at-poke to get the accessible data, browse downto the terminal widget, have a look at caret position.
- in the gnome-terminal, launch vi


Actual results:
In the terminal, the cursor appears at (0,0), but caret position is reported s 1704, i.e. line 25 col 0 !

- press i to go into insert mode, caret position is again wrong
- type a caracter, caret position gets correct
- back-space it, caret position gets wrong again


Expected results:
caret position should be at (0,0) then (1,0) then back (0,0)


Does this happen every time?
yes


Other information:
Regards,
Samuel
Comment 1 Samuel Thibault 2005-05-16 22:35:59 UTC
Here is another situation where the caret position is wrong:

Steps to reproduce:
- run gnome-terminal,
- run at-poke to get the accessible data, browse downto the terminal widget, have a look at caret position.
- in the gnome-terminal, launch dd
- type some spaces

Actual result:
The "Text" property doesn't change, neither does the caret position (of course since else it would get beyond the text...).

Expected result:
The "Text" property should get spaces, and the caret position should be updated as appropriate.

Does this happen every time ?
yes

Regards,
Samuel
Comment 2 Samuel Thibault 2005-05-17 21:06:30 UTC
BTW, I don't understand the comment in vteaccess.c at the end of vte_terminal_accessible_update_private_data_if_needed():          /* If no cells are before the caret, then the caret must be          * at the end of the buffer. */         if (caret == -1) {                 caret = priv->snapshot_characters->len;         }  If no cells are before the caret, that means that the caret is before any cell, i.e. caret is at position *0*. This explains the *really* odd value that my first bug example gives.  --- vteaccess.c	2005-03-09 12:24:56.000000000 +0100 +++ vteaccess.c	2005-05-17 22:47:21.000000000 +0200 @@ -409,7 +409,7 @@  #endif    	/* Get the offsets to the beginnings of each line. */ -	caret = -1; +	caret = 0;  	for (i = 0; i < priv->snapshot_characters->len; i++) {  		/* Get the attributes for the current cell. */  		offset = g_array_index(priv->snapshot_characters, @@ -425,12 +425,6 @@  		}  	}   -	/* If no cells are before the caret, then the caret must be -	 * at the end of the buffer. */ -	if (caret == -1) { -		caret = priv->snapshot_characters->len; -	} -  	/* Notify observers if the caret moved. */  	if (caret != priv->snapshot_caret) {  		priv->snapshot_caret = caret;  will at least give a more appropriate result in the first bug example.  But the problem of blank lines not holding spaces remains and needs solved. 
Comment 3 Samuel Thibault 2005-05-17 21:08:34 UTC
Created attachment 46570 [details] [review]
not-garbaged patch which enhances the first bug example
Comment 4 Luis Villa 2005-07-14 19:05:38 UTC
Bill, are you guys still using zvt?
Comment 5 bill.haneman 2005-07-15 09:46:09 UTC
No, but I think this is a dup of another vte bug (regarding bad caret position),
for which we have a patch.... I could be mistaken.
Comment 6 bill.haneman 2005-07-15 09:48:12 UTC
hmm, not a dup but appears similar to bug 156161
Comment 7 Calum Benson 2006-04-26 17:15:52 UTC
Apologies for spam... ensuring Sun a11y folks are cc'ed on all current accessibility bugs.
Comment 8 Calum Benson 2006-04-26 17:51:42 UTC
Apologies for spam... marking as AP2 to reflect accessibility impact.
Comment 9 Behdad Esfahbod 2007-11-28 12:53:12 UTC
We fixed some a11y caret stuff last year.  Is this bug still valid?
Comment 10 Samuel Thibault 2007-11-28 13:44:03 UTC
I can still reproduce the bug with gnome-terminal 2.18.2, libvte 0.16.9 and atk 1.20.0.  But aren't you able to reproduce it?  I just followed my own steps described above to reproduce it. 
Comment 11 Samuel Thibault 2008-03-02 13:33:46 UTC
More precisely, the "dd" bug got fixed, but not the vi one.
Comment 12 Fabio Durán Verdugo 2010-12-13 02:50:02 UTC
any news for this report?
Comment 13 Samuel Thibault 2011-04-19 22:35:42 UTC
It seems I missed the last ping.

Yes, the bug is still there, and pretty annoying when using any kind of
textmode editor...
Comment 14 Sébastien Hinderer 2011-04-19 22:53:01 UTC
I have this problem too and it would be nice if it could be fixed, eventually...
Comment 15 Jason White 2013-01-02 04:58:34 UTC
Created attachment 232498 [details] [review]
Proposed patch.


This bug is still current. Like Samuel, I don't understand the comment at the
end of the function and the circumstances in which the caret position should
be set to the end of the buffer. Clearly, however, this branch is reached when
it shouldn't be.

Here is a first attempt at a patch. I have tested it and it solves the problem
that arises when the cursor is on the first character position of the
terminal.

Thorough review would be much appreciated.
Comment 16 Jason White 2013-01-03 22:58:52 UTC
Just to clarify the logic of the patch: under the original code, if the cursor
is on the first character of the buffer, then we enter the loop with caret ==
-1, ccol == 0 and crow == attrs.row (where attr hold the attributes of the
first character). Hence attrs.column == ccol and caret is not updated as it
should be. After the loop exits, caret is still -1 and therefore incorrectly
changed to priv->snapshot_characters->len (this also happens, desirably, in the
case of an empty buffer - are there any other cases?)

The patch fixes the bug while avoiding unnecessary entry into the loop.

Note that the cursor's position is sometimes one character beyond the last
character on a line, and so one cannot simply change the test in the loop from
attrs.column < ccol to attrs.column <= ccol and then change the assignment
from caret = i + 1 to caret = i.
Comment 17 Mike Gorse 2013-05-31 15:43:30 UTC
Christian, could you look at the patch that Jason provided please?

It looks okay to me, but it isn't my code.
Comment 18 Mike Gorse 2015-01-21 16:08:26 UTC
I've pushed this to master as commit 3e3e3f. Revert if you object.
Comment 19 Christian Persch 2015-01-21 16:11:58 UTC
I don't see any vte developer accepting this patch; DO NOT commit to vte without authorisation.
Comment 20 Samuel Thibault 2015-01-21 16:18:06 UTC
Well, I don't either see any vte developer seemingly caring about the patch for two years, or having a closer look at the issue for 10 years...
Comment 21 Egmont Koblinger 2015-01-21 18:23:27 UTC
Could you please give updated info on how to reproduce the bug?  at-poke is ancient and no longer shipped by my distro, I doubt it would compile and work.  (I'm not familiar with gnome's a11y.)

This bug is on my "want to review/fix" list (along with the other a11y issues), once I even started looking at it, but fell through the cracks.

Ps. I can't tell at this moment whether the patch's code is correct, but its indentation is definitely not.
Comment 22 Samuel Thibault 2015-01-21 18:44:33 UTC
Well, replace "at-poke" with "accerciser" in the original report, and the rest still applies.
Comment 23 Samuel Thibault 2015-01-21 18:46:57 UTC
More precisely, after having found the Terminal widget in the tree, its caret position can be found in the Interface Viewer tab, Text subtab, it's called "Offset" there.
Comment 24 Mike Gorse 2015-01-22 00:56:29 UTC
Created attachment 295139 [details]
Pyatspi script to watch for caret-moved events.

I made this pyatspi script in case it's helpful. It will print out the reported cursor position when an object:text-caret-moved event is sent. If you run it in a terminal, then do something in another terminal that moves the cursor to the upper left corner of the screen (ie, start vi), you will see the wrong value for the location of the cursor. Or you can do what Samuel suggests using accerciser.
Comment 25 Egmont Koblinger 2015-01-25 16:54:41 UTC
Thanks for caret.py, it seems to print the correct values.  The patch indeed seems to solve the problem.

Note: caret.py itself should be ran in a non-gnome-based terminal, so that it doesn't include cursor changes occurring in that terminal where caret.py itself is running.

accerciser doesn't report the correct position though, it reports the text length instead.  After starting vim, it always prints 1944 (= 81x24, in case of 80x24 terminal size).  The Start and End values shown don't seem to many any sense either.
Comment 26 Egmont Koblinger 2015-01-25 17:48:11 UTC
I like the first patch (by Samuel, comment 3).  It's clean, easily verifiable, and does the right thing.

It could be further improved by breaking out of the loop when the condition "If this cell is "before" the cursor[...]" fails as it can't become true again later on in the same loop.

I don't like the second patch (by Jason, comment 15).  It's longer (the final code is 19 lines longer than Samuel's), more complicated, and handles cursor in the upper left corner as a special case although I don't see any reason it should be special.

Is there any issue that's fixed by the second patch but not by the first?
Comment 27 Mike Gorse 2015-01-31 15:09:49 UTC
(In reply to comment #26)

> Is there any issue that's fixed by the second patch but not by the first?

Not that I know of. I hadn't noticed the first patch. I just tested it, and it also fixes the issue for meThanks for looking at this bug.
Comment 28 Egmont Koblinger 2015-02-01 19:43:31 UTC
Thanks guys!  I've committed the original patch.

I didn't add optimization, but made a note in bug 734195.