GNOME Bugzilla – Bug 754964
backspacing over space produces bogus events
Last modified: 2015-10-07 20:36:01 UTC
Created attachment 311233 [details] fix Hello, We are having issues with the brltty screen reading of gnome-terminal, and it seems the bug is in VTE. To reproduce, run cat without parameters, then type space, then backspace, space, backspace, space, backspace, etc. The AT-SPI signals emitted by VTE will be insert " ", caret move to second character, then delete " " and caret move to first character, then caret move to second character (with no insert), then delete " " and caret move to first character, etc. The result is that brltty rightfully removes the leading text character by character, up to the end of the string. It then produces very odd results when reading the rest of the screen. VTE should either insert the spaces again, or just not delete them. The attached patch disables what is apparently emitting the delete events. With that applied, the emitted signals are insert " ", caret move to second character, then caret move to first character, then caret move to second character, then caret move to first character, etc. and the result in brltty becomes correct.
Looks good to me. ChPe? Just for the record, and to make sure we're on the same page: When pressing backspace at a "cat" (even without a11y), the character is visually removed by the kernel emitting a "backspace"+"space"+"backspace" combo. In your descriptions (both the buggy and the fixed case) the first «insert " ", caret move to second character"» refers to manually pressing the space. Later, that space is replaced by another kernel-emitted space when you press the backspace key, but after all, the onscreen contents (including the trailing spaces which remain there) remains the same, so only cursor movements should be generated. The special case would make sense if there was a special case for shortening the line (automatically truncating the trailing whitespaces) as well as printing a space was also special cased (delayed until followed by a real char). All these are probably not worth the hassle. Just handle space as any other character.
Yes, I agree with all of that :)
Cool! Thanks for the patch!
History says this was added for bug 150858. Does this patch regress that problem (if it still a valid problem with orca instead of gnopernicus)?
I see... So vte was made to report text changes with speech in mind. It is true that for speech it makes sense to talk the dropped space. But that completely breaks keeping the text data up to date in the screen reader: the space character is *not* removed from the text content, and e.g. caret positions after that dropped character would become wrong. I'll have a look at emitting deletion + insertion of the space character (which is stupid from the point of view of brltty, but makes sense for speech synthesis).
Created attachment 311242 [details] [review] proposed fix Here is a fix proposal: it simply reinserts the just-removed space. This fixes the output in brltty, and orca's behavior remains the space: it says "space".
At this point I'm lost and I'm not confident enough to cast a vote on this change right before the feature freeze. I'd need more time to understand what's going on, which I don't have at the moment. But ChPe you might go ahead if you like the patch. Or we can wait a bit.
well, my first fix was to remove the extraneous delete, since as you said the content doesn't actually change when backspacing over a space. My second fix is to keep the delete, but add an insert right after it, so as to cancel the effect of the insert, and thus making the whole thing a nop in the end, while still emitting delete/insert events for orca to announce them.
I'd also rather not apply this right before hard code freeze starts. I'll look at this for 0.42.1.
Well, I don't see why delaying applying this fix. This bug makes VTE barely usable for people using brltty to work in terminals (which is way more convenient than orca for that), since ^H over a space happens very often in applications. My understanding of code freeze is stopping adding new features and only fix bugs. This is not a feature, but really a bug fix.
(In reply to Samuel Thibault from comment #10) > My understanding of code freeze is stopping adding new > features and only fix bugs. That's called "feature freeze". "Code freeze" means code freeze: no changes at all, not even bugfixes (unless something critical is discovered, but even that requires special approvals). Don't worry: you just reported this bug today (although presumably it's been there for ~10 years), and the fix will make it into 3.18.1 released in a month from now. It's not bad, is it? :)
Ah, so there's a dot release, good.
Ping?
Ok to commit to master and 0-42. (Please use a commit message describing the issue and linking to this bug; or if you don't have commit access, attach a git-format-patch formatted patch here.)
Created attachment 312452 [details] [review] proposed fix I don't have commit access, here is a git format-patch