Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase default caret move timeout from 30 to 100 ms and make it configurable #7201

Merged
merged 5 commits into from
Jul 10, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions source/IAccessibleHandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,8 +612,12 @@ def processGenericWinEvent(eventID,window,objectID,childID):
appModuleHandler.update(winUser.getWindowThreadProcessID(window)[0])
#Handle particular events for the special MSAA caret object just as if they were for the focus object
focus=eventHandler.lastQueuedFocusObject
if focus and objectID==winUser.OBJID_CARET and eventID in (winUser.EVENT_OBJECT_LOCATIONCHANGE,winUser.EVENT_OBJECT_SHOW):
NVDAEvent=("caret",focus)
if objectID==winUser.OBJID_CARET and eventID in (winUser.EVENT_OBJECT_LOCATIONCHANGE,winUser.EVENT_OBJECT_SHOW):
if isinstance(focus,NVDAObjects.window.Window) and focus.shouldAllowSystemCaretEvent:
NVDAEvent=("caret",focus)
else:
# No focus or the focus doesn't want these caret events.
return
else:
NVDAEvent=winEventToNVDAEvent(eventID,window,objectID,childID)
if not NVDAEvent:
Expand Down
6 changes: 6 additions & 0 deletions source/NVDAObjects/IAccessible/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,12 @@ def _get_TextInfo(self):
return IA2TextTextInfo
return super(IAccessible,self).TextInfo

def _get_shouldAllowSystemCaretEvent(self):
if hasattr(self, 'IAccessibleTextObject'):
# IAccessibleText implementations fire IA2 caret events which are more reliable.
return False
return super(IAccessible, self).shouldAllowSystemCaretEvent

def _isEqual(self,other):
if self.IAccessibleChildID!=other.IAccessibleChildID:
return False
Expand Down
4 changes: 4 additions & 0 deletions source/NVDAObjects/window/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,10 @@ def _get_devInfo(self):
info.append("displayText: %s" % ret)
return info

#: Whether to allow caret events generated by the system caret;
#: i.e. as triggered by the Win32 functions ShowCaret and SetCaretPos.
shouldAllowSystemCaretEvent = True

class Desktop(Window):

isPresentableFocusAncestor = False
Expand Down
5 changes: 4 additions & 1 deletion source/config/configSpec.py
Original file line number Diff line number Diff line change
@@ -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.

Expand Down Expand Up @@ -192,6 +192,9 @@

[upgrade]
newLaptopKeyboardLayout = boolean(default=false)

[editableText]
caretMoveTimeoutMs = integer(min=0, max=2000, default=100)
""").format(latestSchemaVersion=latestSchemaVersion)

#: The configuration specification
Expand Down
58 changes: 46 additions & 12 deletions source/editableText.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -42,37 +43,70 @@ 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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the description of this PR you mention backwards compat for this function I think? Changing timeout to None will risk breaking that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's certainly a valid point, but I think it's a negligible/acceptable risk.

  1. It's most likely that a caller would call this function either accepting the defaults or passing its own timeout, both of which will still work as expected.
  2. This doesn't call super, so there's no chance that some method will receive None that wasn't expecting it.
  3. The only risk I can think of is that a subclass might have hard-coded the old 0.03 default, in which case they won't get the user's configured value. However, at worst, this means some subclass won't behave consistently with the rest of NVDA, and this would be pretty clear from the debug log if caret movement started timing out. I also think the likelihood of someone having done this is unlikely.
  4. Since it's a private method, maintaining backwards compat is a courtesy anyway, not an obligation. :)

"""
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
@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
if eventHandler.isPendingEvents("caret"):
log.debug("caret event. Elapsed: %d ms" % elapsed)
return (True, newInfo)
if eventHandler.isPendingEvents("valueChange"):
# When pressing delete, the caret might not physically move,
# so the control might not fire a caret event,
# even though the text under the caret changes.
# We still want to treat this as a caret move.
# Otherwise, the user will have to wait
# the full timeout for feedback after pressing delete.
log.debug("valueChange event. Elapsed: %d ms" % elapsed)
return (True, newInfo)
# Some controls don't fire caret events.
# 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 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):
Expand Down