GNOME Bugzilla – Bug 166637
Accessible's caret position sometimes wrong
Last modified: 2015-02-01 19:43:31 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
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
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.
Created attachment 46570 [details] [review] not-garbaged patch which enhances the first bug example
Bill, are you guys still using zvt?
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.
hmm, not a dup but appears similar to bug 156161
Apologies for spam... ensuring Sun a11y folks are cc'ed on all current accessibility bugs.
Apologies for spam... marking as AP2 to reflect accessibility impact.
We fixed some a11y caret stuff last year. Is this bug still valid?
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.
More precisely, the "dd" bug got fixed, but not the vi one.
any news for this report?
It seems I missed the last ping. Yes, the bug is still there, and pretty annoying when using any kind of textmode editor...
I have this problem too and it would be nice if it could be fixed, eventually...
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.
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.
Christian, could you look at the patch that Jason provided please? It looks okay to me, but it isn't my code.
I've pushed this to master as commit 3e3e3f. Revert if you object.
I don't see any vte developer accepting this patch; DO NOT commit to vte without authorisation.
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...
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.
Well, replace "at-poke" with "accerciser" in the original report, and the rest still applies.
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.
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.
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.
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?
(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.
Thanks guys! I've committed the original patch. I didn't add optimization, but made a note in bug 734195.