GNOME Bugzilla – Bug 372684
GTerminalPerk improvements
Last modified: 2007-01-16 19:49:32 UTC
I think we can correct the reporting of lines of text in the terminal by watching the offsets on move events and deciding whether to allow a following insert or delete event. Alternatively, perhaps we can correct the event ordering and pass the correct information to the DefaultPerk. Either way, it's time to do something about the problems in the terminal.
The problem here is the following. All well behaved gtk text boxes fire events in this order: Insertion 1) insert 2) move Deletion 1) delete (delete key) or 1) delete (backspace key) 2) move For the gnome-terminal, we get a crazy ordering of events: Insertion 1) move (to the location one past the last character in the text to be inserted) 2) delete (all text to be inserted before it's inserted) 3) insert (the text to be inserted) Deletion 1) delete (delete key) 2) insert or 1) move 2) delete (backspace key) 3) insert I've reported this to the vte folks as bug #372777. I don't expect them to fix the problem anytime soon. In the meantime, we need to fix the problem in a gnome-terminal script. The fix for bug #373400 gives us extra information about the last key pressed that can help us ignore the rouge insert/delete events in the GTerminalPerk. Ideally, the gnome-terminal Perk would simply correct the event order so that it matches the standard gtk order and then just let the DefaultPerk (and BasicBraillePerk in the future) "do the right thing" when reporting caret events. If that's not possible, then GTerminalPerk will have to do its own output to fix any problems.
Created attachment 79195 [details] [review] gterminal fixes This usually gets the event ordering right, but the text_offset is still curius and needs to be fixed.
Comment on attachment 79195 [details] [review] gterminal fixes Some comments on the patch: * Don't want the timer task running when the terminal is not focused. The task will have to registered and unregistered on focus/unfocus. * Key syms should probably be used for comparison of last key press instead of hex codes. The key syms are fixed English names of keys that should not change.
Created attachment 80213 [details] [review] Proposed patch I think this perk is less trivial then we hoped. There are two major obstacles that still exist. The most significant one is the CaretTask's update() method in BasicSpeechPerk which confuses BasicSpeechperk as to what was the last text offset. There should be a way to block the update method too. Or maybe CHAIN_AROUND? The second part is that gterminal does not seem to inform what text was deleted in a "delete" event. So the current perk does some bad guesswork.
Created attachment 80296 [details] [review] gterminal fixes Small cleanups
So do you want to block the updating of the caret_por in BasicSpeechPerk? What information is it caching that is considered "bad" when the terminal Perk executes?
Created attachment 80367 [details] [review] gterminal fixes Bigger fixes.
I think this patch is ready for review.
Eitan, I experience the following problems when running with this patch: 1) This exception occurred once: [Tier ERROR 2007-01-16 08:53:49,478] MoveTimer: 0 {} Traceback (most recent call last):
+ Trace 102687
rv = task.execute(layer=layer, **params)
for task_info in self.perk.caret_ev_seq.sequenceReorderIter(op_name):
yield self.caret_events.get(op)[0]
2) CPU usage sits around 50%. Perhaps the timer is on too short an interval? Please make these fixes top priority today. I will continue reviewing your patch throughout the day.
Created attachment 80413 [details] [review] gterminal fixes This should hopefully address 1. As for 2, were talking about it now on IRC, and it seems to be a timer bug.
Created attachment 80420 [details] [review] minor updates to Eitan's code Adds check for focus on accessible with role terminal to prevent processing in other text fields in gnome-terminal. Moves some initialization logic from the task to the perk. Adds some minor comments.
Fixed in the development version. The fix will be available in the next major release. Thank you for your bug report.