Increase default caret move timeout from 30 to 100 ms and make it con…

…figurable (PR #7201, issue #6424)

- 30 ms was chosen to ensure maximum responsiveness, but this just isn't enough in the wild; e.g. in complex web editors (particularly in Chrome) or when connecting to remote terminals with connection latency.
- This is configurable via config.conf["editableText"]["caretMoveTimeout"]. It is not exposed in the GUI, as this should only be changed by advanced users. The value is in ms, not seconds, as that's a better unit for this purpose. However, the function continues to use seconds for its arguments for backwards compatibility.
- Previously, _hasCaretMoved slept at the end of the loop but didn't check for caret movement again. This meant that it could return False even if the caret did actually move during the last sleep. Now, the timeout check happens after the caret movement check, ensuring the correct result.
- _hasCaretMoved now works with ms rather than seconds to avoid floating point precision errors. These precision errors could result in an extraneous additional retry.
- Added some debug logging to help diagnose caret tracking problems in future.
- When pressing the delete key, we often can't detect this by comparing bookmarks. Therefore, when pressing delete, compare the word at the caret (in addition to bookmarks) to detect movement.

source/config/configSpec.py

@@ -1,6 +1,6 @@
 # -*- coding: UTF-8 -*-
 #A part of NonVisual Desktop Access (NVDA)
-#Copyright (C) 2006-2016 NV Access Limited
+#Copyright (C) 2006-2017 NV Access Limited
 #This file is covered by the GNU General Public License.
 #See the file COPYING for more details.
 
@@ -194,6 +194,9 @@
 
 [upgrade]
 	newLaptopKeyboardLayout = boolean(default=false)
+
+[editableText]
+	caretMoveTimeoutMs = integer(min=0, max=2000, default=100)
 """).format(latestSchemaVersion=latestSchemaVersion)
 
 #: The configuration specification

source/editableText.py

@@ -2,7 +2,7 @@
 #A part of NonVisual Desktop Access (NVDA)
 #This file is covered by the GNU General Public License.
 #See the file COPYING for more details.
-#Copyright (C) 2006-2012 NV Access Limited
+#Copyright (C) 2006-2017 NV Access Limited
 
 """Common support for editable text.
 @note: If you want editable text functionality for an NVDAObject,
@@ -21,6 +21,7 @@
 from scriptHandler import isScriptWaiting, willSayAllResume
 import textInfos
 import controlTypes
+from logHandler import log
 
 class EditableText(ScriptableObject):
 	"""Provides scripts to report appropriately when moving the caret in editable text fields.
@@ -42,37 +43,69 @@ class EditableText(ScriptableObject):
 	#: Whether or not to announce text found before the caret on a new line (e.g. auto numbering)
 	announceNewLineText=True
 
-	def _hasCaretMoved(self, bookmark, retryInterval=0.01, timeout=0.03):
+	def _hasCaretMoved(self, bookmark, retryInterval=0.01, timeout=None, origWord=None):
 		"""
 		Waits for the caret to move, for a timeout to elapse, or for a new focus event or script to be queued.
 		@param bookmark: a bookmark representing the position of the caret before  it was instructed to move
 		@type bookmark: bookmark
 		@param retryInterval: the interval of time in seconds this method should  wait before checking the caret each time.
 		@type retryInterval: float 
-		@param timeout: the over all amount of time in seconds the method should wait before giving up completely.
+		@param timeout: the over all amount of time in seconds the method should wait before giving up completely,
+			C{None} to use the value from the configuration.
 		@type timeout: float
+		@param origWord: The word at the caret before the movement command,
+			C{None} if the word at the caret should not be used to detect movement.
+			This is intended for use with the delete key.
 		@return: a tuple containing a boolean denoting whether this method timed out, and  a TextInfo representing the old or updated caret position or None if interupted by a script or focus event.
 		@rtype: tuple
- 		"""
+		"""
+		if timeout is None:
+			timeoutMs = config.conf["editableText"]["caretMoveTimeoutMs"]
+		else:
+			# This function's arguments are in seconds, but we want ms.
+			timeoutMs = timeout * 1000
+		# time.sleep accepts seconds, so retryInterval is in seconds.
+		# Convert to integer ms to avoid floating point precision errors when adding to elapsed.
+		retryMs = int(retryInterval * 1000)
 		elapsed = 0
 		newInfo=None
-		while elapsed < timeout:
+		while True:
 			if isScriptWaiting():
 				return (False,None)
 			api.processPendingEvents(processEventQueue=False)
 			if eventHandler.isPendingEvents("gainFocus"):
+				log.debug("Focus event. Elapsed: %d ms" % elapsed)
 				return (True,None)
-			#The caret may stop working as the focus jumps, we want to stay in the while loop though
+			# If the focus changes after this point, fetching the caret may fail,
+			# but we still want to stay in this loop.
 			try:
 				newInfo = self.makeTextInfo(textInfos.POSITION_CARET)
-				newBookmark = newInfo.bookmark
 			except (RuntimeError,NotImplementedError):
-				newInfo=None
-			else:
-				if newBookmark!=bookmark:
-					return (True,newInfo)
+				newInfo = None
+			# Caret events are unreliable in some controls.
+			# Try to detect with bookmarks.
+			newBookmark = None
+			if newInfo:
+				try:
+					newBookmark = newInfo.bookmark
+				except (RuntimeError,NotImplementedError):
+					pass
+			if newBookmark and newBookmark!=bookmark:
+				log.debug("Caret move detected using bookmarks. Elapsed: %d ms" % elapsed)
+				return (True, newInfo)
+			if origWord is not None and newInfo:
+				# When pressing delete, bookmarks might not be enough to detect caret movement.
+				wordInfo = newInfo.copy()
+				wordInfo.expand(textInfos.UNIT_WORD)
+				word = wordInfo.text
+				if word != origWord:
+					log.debug("Word at caret changed. Elapsed: %d ms" % elapsed)
+					return (True, newInfo)
+			if elapsed >= timeoutMs:
+				break
 			time.sleep(retryInterval)
-			elapsed += retryInterval
+			elapsed += retryMs
+		log.debug("Caret didn't move before timeout. Elapsed: %d ms" % elapsed)
 		return (False,newInfo)
 
 	def _caretScriptPostMovedHelper(self, speakUnit, gesture, info=None):
@@ -198,9 +231,11 @@ def script_caret_delete(self,gesture):
 			gesture.send()
 			return
 		bookmark=info.bookmark
+		info.expand(textInfos.UNIT_WORD)
+		word=info.text
 		gesture.send()
 		# We'll try waiting for the caret to move, but we don't care if it doesn't.
-		caretMoved,newInfo=self._hasCaretMoved(bookmark)
+		caretMoved,newInfo=self._hasCaretMoved(bookmark,origWord=word)
 		self._caretScriptPostMovedHelper(textInfos.UNIT_CHARACTER,gesture,newInfo)
 		braille.handler.handleCaretMove(self)