GNOME Bugzilla – Bug 398006
vte_terminal_emit_status_line_changed called per unichar
Last modified: 2014-05-18 18:04:08 UTC
vte_terminal_emit_status_line_changed() is called for every unichar during process_incoming unlike the other signal emission routines.
Created attachment 80595 [details] [review] Move the signal emission to an idle handler.
Now that we run vte_terminal_process_incoming() multiple times per display, perhaps the text-{changed,deleted,modified} should also be push to idle functions.
Created attachment 80767 [details] [review] Queue all the text signal emission.
No idea on this one. Mariano? Rich, can you test with this patch and make sure that it doesn't make Orca unhappy?
Created attachment 80774 [details] [review] Queue *all* 'text' signals This goes even further and moves any signal that looked like it could be emitted by vte_terminal_process_incoming() into a common idle handler.
Perhaps the biggest issue is that it does not help in resolving: Bug 372777 – Wrong, out of order accessibility text events
Hehdad, I can test on Monday, but I'm cc:'ing in Will Walker now. Moving this stuff to an idle handler sounds like the wrong thing to do from an accessibility standpoint if the terminal is generating a lot of continuous output (like the results of an ./autogen.sh run). Will, thoughts? ...
s/Hehdad/Behdad/ Sorry, I was in a hurry.
What does this get us?
Elegance. :-) I'll do some profiling with ally enabled at some point...
(In reply to comment #7) > Behdad, I can test on Monday, but I'm cc:'ing in Will Walker now. > Moving this stuff to an idle handler sounds like the wrong thing > to do from an accessibility standpoint if the terminal is generating > a lot of continuous output (like the results of an ./autogen.sh run). > Will, thoughts? ... > I think accessible events are meant to be synchronous. Putting things in a queue means losing any assumption about object state that we might make in a synchronous callback. Having said that, Orca tries not to make such assumptions, and queues up all events for later processing. As such, I'm not sure Orca will lose much by the proposed patch. Other assistive technologies might not queue events, however, and might make assumptions about being called synchronously. The ultimate decider for whether or not event callbacks should occur at the time of the event would be Bill, though. I'll add him to the CC for this bug.
Ok, AIUI the current situtation in vte (0.15.1) is effectively that one text-{inserted,modified,deleted} gets emitted per display. HEAD now tries to run vte_terminal_process_incoming() multiple times before displaying which will result in multiple signal emissions per display. Moving those emissions to the idle handler will ensure that there is just a single signal per display and that the content during the signal exactly matches what is being displayed. Due to the 40Hz display rate limit we can be confident that the idle signal handler will be emitted first (though we could adjust the priorities to be certain). Are there any good resources that explain the "dos and don'ts" of ally?
Would it be OK to create and call vte_terminal_process_incoming_final(), to emit these signals once, synchronously? "delete" needs to be called synchronously for sure, since otherwise the contract can't be fulfilled (the event encapsulates the deleted string - how would you obtain that in the idle handler?) Similarly for 'add'/'replace'. See comment #6 - any changes to the text event emission need to address known problems IMO, this one will create new ones!
The use of an additional idle handler is indeed superfluous and can be resolved with a vte_terminal_process_incoming_final() as you point out. Ok, I'll try and address bug 372777 first.
Following a discussion with ChPe today, I've removed the status line support. We're aiming for xterm compability. Xterm doesn't support this, so probably most users haven't had access to this feature anyway. We'd need to implement more features, as described in http://vt100.net/docs/vt510-rm/DECSASD and http://vt100.net/docs/vt510-rm/DECSSDT. Restricted scroll mode offers an easy way to achieve something similar, and ncurses apps easily have control over the entire screen. Probably nobody was using this feature.