Python 3: Fix error with accPropServer

source/gui/accPropServer.py

@@ -5,16 +5,30 @@
 #See the file COPYING for more details.
 
 """Implementation of IAccProcServer, so that customization of a wx control can be done very fast."""
+from ctypes.wintypes import BOOL
+from typing import Optional, Tuple, Any, Union, Callable
 
 from logHandler import log
-from  comtypes.automation import VT_EMPTY
-from  comtypes import COMObject, GUID
+from comtypes.automation import S_OK, VARIANT, POINTER, c_int, c_double, _oleaut32
+from comtypes import COMObject, GUID
 from comInterfaces.Accessibility import IAccPropServer, ANNO_CONTAINER, ANNO_THIS
-from abc import ABCMeta, abstractmethod, abstractproperty
+from abc import ABCMeta, abstractmethod
 import weakref
 import winUser
 import wx
 
+_VariantInit: Callable[[POINTER(VARIANT),], None] = _oleaut32.VariantInit
+_VariantInit.argtypes = (POINTER(VARIANT),)
+
+AcceptedGetPropTypes = Union[
+		bool,
+		int, c_int,
+		float, c_double,
+		str,
+		VARIANT,
+		# And others: L{comtpyes.automation.tagVariant._set_value}
+	]
+
 class IAccPropServer_Impl(COMObject, metaclass=ABCMeta):
 	"""Base class for implementing a COM interface for a hwnd based AccPropServer\
 	to annotate a WX control.
@@ -35,10 +49,6 @@ class IAccPropServer_Impl(COMObject, metaclass=ABCMeta):
 	# https://msdn.microsoft.com/en-us/library/windows/desktop/dd318495(v=vs.85).aspx
 	HAS_PROP = 1  # TRUE - Constant for `BOOL* pfHasProp` out param of `IAccPropServer::GetPropValue` method
 	DOES_NOT_HAVE_PROP = 0  # FALSE - Constant for `BOOL* pfHasProp` out param of `IAccPropServer::GetPropValue` method
-	# When returning `DOES_NOT_HAVE_PROP` or FALSE as the pfHasProp part of the return of `IAccPropServer::GetPropValue`
-	# method, then `pvarValue` return value must be `VT_EMPTY`.
-	# Consider using `NO_RETURN_VALUE`
-	NO_RETURN_VALUE = (VT_EMPTY, DOES_NOT_HAVE_PROP)
 
 	# An array with the GUIDs of the properties that an AccPropServer should override
 	properties_GUIDPTR = []
@@ -78,8 +88,14 @@ def __init__(self, control, annotateProperties, annotateChildren=False):
 		control.Bind(wx.EVT_WINDOW_DESTROY, self._onDestroyControl, source=control)
 
 	@abstractmethod
-	def _getPropValue(self, pIDString, dwIDStringLen, idProp):
-		"""use this method to implement GetPropValue. It  is wrapped by the callback GetPropValue to handle exceptions.
+	def _getPropValue(
+			self,
+			pIDString: str,
+			dwIDStringLen: int,
+			idProp: GUID
+	) -> Optional[Tuple[BOOL, AcceptedGetPropTypes]]:
+		""" Use this method to implement GetPropValue.
+		It is wrapped by the callback GetPropValue to handle exceptions, and ensure valid return types.
 		For instructions on implementing accPropServers, see https://msdn.microsoft.com/en-us/library/windows/desktop/dd373681(v=vs.85).aspx .
 		For instructions specifically about this method, see https://msdn.microsoft.com/en-us/library/windows/desktop/dd318495(v=vs.85).aspx .
 		@param pIDString: Contains a string that identifies the property being requested.
@@ -90,24 +106,58 @@ def _getPropValue(self, pIDString, dwIDStringLen, idProp):
 			to extract the HWND/idObject/idChild from the identity string.
 			Note that, while one IAccPropServer implementation can annotate
 			multiple accessible elements, it is still bound to one wx.Control.
-		@type pIDString: str
 		@param dwIDStringLen: Specifies the length of the identity string specified by the pIDString parameter.
-		@type dwIDStringLen: int
-		@param idProp: Specifies a GUID indicating the desired property.
-		@type idProp: One of the oleacc.PROPID_* GUIDS
-		@return A tuple of the out params for the `IAccPropServer::GetPropValue` method: `VARIANT* pvarValue` and `BOOL*
-		pfHasProp`. When the pfHasProp part is FALSE / self.DOES_NOT_HAVE_PROP, then the pvarValue part must be VT_EMPTY.
-		Consider using self.NO_RETURN_VALUE instead. Returning (VT_EMPTY, HAS_PROP) IS valid, meaning the property exists
-		but is empty.
+		@param idProp: Specifies a GUID indicating the desired property. One of the values from oleacc.PROPID_*
+		@return Use L{self._hasProp} to return correct values or return None if unable to supply the property.
 		"""
 		raise NotImplementedError
 
-	def GetPropValue(self, pIDString, dwIDStringLen, idProp):
+	def _hasProp(
+			self,
+			value: AcceptedGetPropTypes
+	) -> Optional[Tuple[BOOL, AcceptedGetPropTypes]]:
+		"""Constructs a tuple for the `IAccPropServer::GetPropValue` method, two elements:
+			1. `VARIANT pvarValue`
+			2. `BOOL pfHasProp` (either self.HAS_PROP or self.DOES_NOT_HAVE_PROP)"""
+		return value, self.HAS_PROP
+
+	def GetPropValue(
+			self, this, # unused "this" used to indicate to comTypes we want a low level implementation
+			pIDString: str,
+			dwIDStringLen: int,
+			idProp: GUID,
+			pvarValue: POINTER(VARIANT),
+			pfGotProp: POINTER(BOOL)
+	) -> int:
+		""" Exposed method to get a prop value.
+		see L{_getPropValue} for more details of args.
+		Uses a low-level approach, because comtypes tries to clear the VARIANT even though it is an out param.
+		When the pfHasProp part is FALSE / self.DOES_NOT_HAVE_PROP, then the pvarValue.vt part must be VT_EMPTY.
+		"""
+		# ensure exceptions don't leave this function. They will get get swallowed by the caller.
+		# instead catch and log exceptions.
 		try:
-			return self._getPropValue(pIDString, dwIDStringLen, idProp)
-		except Exception:
+			# Preset values for "no prop value", in case we return early.
+			pfGotProp.contents.value = self.DOES_NOT_HAVE_PROP
+			_VariantInit(pvarValue)
+
+			ret = self._getPropValue(pIDString, dwIDStringLen, idProp)
+			if ret is None:
+				# We don't have the prop value, return early.
+				return S_OK
+			elif len(ret) != 2:
+				# We don't have the prop value, internal error.
+				raise RuntimeError("_getPropValue implementation must return None or two element tuple")
+			elif ret[1] != self.HAS_PROP:
+				# We don't have the prop value, return early.
+				return S_OK
+
+			# we do have the prop value
+			pfGotProp.contents.value = self.HAS_PROP
+			pvarValue.contents.value = ret[0]
+		except Exception as e: # catch and log all exceptions so they are not swallowed by caller.
 			log.exception()
-			return self.NO_RETURN_VALUE
+		return S_OK
 
 	def _onDestroyControl(self, evt):
 		evt.Skip()  # Allow other handlers to process this event.

source/gui/nvdaControls.py

@@ -3,8 +3,11 @@
 #Copyright (C) 2016-2018 NV Access Limited, Derek Riemer
 #This file is covered by the GNU General Public License.
 #See the file COPYING for more details.
+from ctypes.wintypes import BOOL
+from typing import Any, Tuple, Optional
 
 import wx
+from comtypes import GUID
 from wx.lib.mixins import listctrl as listmix
 from gui import accPropServer
 from gui.dpiScalingHelper import DpiScalingHelperMixin
@@ -99,17 +102,17 @@ def __init__(self, control, propertyAnnotations):
 	def _getPropValue(self, pIDString, dwIDStringLen, idProp):
 		control = self.control()  # self.control held as a weak ref, ensure it stays alive for the duration of this method
 		if control is None or not self.propertyAnnotations:
-			return self.NO_RETURN_VALUE
+			return None
 
 		try:
 			val = self.propertyAnnotations[idProp]
 			if callable(val):
 				val = val()
-			return val, self.HAS_PROP
+			return self._hasProp(val)
 		except KeyError:
 			pass
 
-		return self.NO_RETURN_VALUE
+		return None
 
 	def _cleanup(self):
 		# could contain references (via lambda) of our owner, set it to None to avoid a circular reference which
@@ -130,19 +133,19 @@ def __init__(self, control):
 			annotateChildren=True
 		)
 
-	def _getPropValue(self, pIDString, dwIDStringLen, idProp):
+	def _getPropValue(self, pIDString: str, dwIDStringLen: int, idProp: GUID) -> Optional[Tuple[BOOL, Any]]:
 		control = self.control()  # self.control held as a weak ref, ensure it stays alive for the duration of this method
 		if control is None:
-			return self.NO_RETURN_VALUE
+			return None
 
 		# Import late to prevent circular import.
 		from IAccessibleHandler import accPropServices
 		handle, objid, childid = accPropServices.DecomposeHwndIdentityString(pIDString, dwIDStringLen)
 		if childid == winUser.CHILDID_SELF:
-			return self.NO_RETURN_VALUE
+			return None
 
 		if idProp == oleacc.PROPID_ACC_ROLE:
-			return oleacc.ROLE_SYSTEM_CHECKBUTTON, self.HAS_PROP
+			return self._hasProp(oleacc.ROLE_SYSTEM_CHECKBUTTON)
 
 		if idProp == oleacc.PROPID_ACC_STATE:
 			states = oleacc.STATE_SYSTEM_SELECTABLE|oleacc.STATE_SYSTEM_FOCUSABLE
@@ -152,7 +155,7 @@ def _getPropValue(self, pIDString, dwIDStringLen, idProp):
 				# wx doesn't seem to  have a method to check whether a list item is focused.
 				# Therefore, assume that a selected item is focused,which is the case in single select list boxes.
 				states |= oleacc.STATE_SYSTEM_SELECTED | oleacc.STATE_SYSTEM_FOCUSED
-			return states, self.HAS_PROP
+			return self._hasProp(states)
 
 class CustomCheckListBox(wx.CheckListBox):
 	"""Custom checkable list to fix a11y bugs in the standard wx checkable list box."""