Skip to content

Commit b3cc309

Browse files
feerrenrutfeerrenrut
authored
Fix Excel error due to state enum changes (PR #13465)
Summary: With UIA for Excel disabled, navigating to a cell with a formula and "has formula" was not reported. This is a regression caused by #13414 In `NVDAHelper/remote/excel.cpp` the properties of a cell were determined and bits were set on the `state` member of a `EXCEL_CELLINFO` struct. The constants used for this are in `NVDAHelper/remote/excel/Constants.h`, see the `NVSTATE_*` constants, previously these matched the `controlTypes.State` constants directly. Description of change: Rather than couple the excel implementation to the controlTypes implementation, these constants have been converted to enums (both in C++ and in Python), renumbered, and an explicit mapping to the corresponding `controlTypes.State` enum has been created. Fixes #13457
1 parent f94a60b commit b3cc309

6 files changed

Lines changed: 112 additions & 40 deletions

File tree

‎nvdaHelper/interfaces/nvdaInProcUtils/nvdaInProcUtils.idl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ typedef struct {
3434
BSTR address;
3535
BSTR inputTitle;
3636
BSTR inputMessage;
37-
hyper states;
37+
hyper nvCellStates; // bitwise OR of the NvCellState enum values that apply to this cell.
3838
long rowNumber;
3939
long rowSpan;
4040
long columnNumber;

‎nvdaHelper/remote/excel.cpp‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ long getCellTextWidth(HWND hwnd, IDispatch* pDispatchRange) {
110110
}
111111

112112
__int64 getCellStates(HWND hwnd, IDispatch* pDispatchRange) {
113-
__int64 states=0;
113+
std::int64_t nvCellStates = 0;
114114
// If the current row is a summary row, expose the collapsed or expanded states depending on wither the inner rows are showing or not.
115115
CComPtr<IDispatch> pDispatchRow=nullptr;
116116
HRESULT res=_com_dispatch_raw_propget(pDispatchRange,XLDISPID_RANGE_ENTIREROW,VT_DISPATCH,&pDispatchRow);
@@ -129,11 +129,13 @@ __int64 getCellStates(HWND hwnd, IDispatch* pDispatchRange) {
129129
if(FAILED(res)) {
130130
LOG_DEBUGWARNING(L"row.showDetail failed with code "<<res);
131131
}
132-
states|=(showDetail?NVSTATE_EXPANDED:NVSTATE_COLLAPSED);
132+
nvCellStates |= (showDetail ? NvCellState::EXPANDED : NvCellState::COLLAPSED);
133133
}
134134
}
135-
// If this row was neither collapsed or expanded, then try the same for columns instead.
136-
if(!(states&NVSTATE_EXPANDED)&&!(states&NVSTATE_COLLAPSED)) {
135+
if( // Row not collapsed or expanded, try the same for columns instead.
136+
!(nvCellStates& NvCellState::EXPANDED)
137+
&&!(nvCellStates & NvCellState::COLLAPSED)
138+
) {
137139
CComPtr<IDispatch> pDispatchColumn=nullptr;
138140
res=_com_dispatch_raw_propget(pDispatchRange,XLDISPID_RANGE_ENTIRECOLUMN,VT_DISPATCH,&pDispatchColumn);
139141
if(FAILED(res)) {
@@ -151,7 +153,7 @@ __int64 getCellStates(HWND hwnd, IDispatch* pDispatchRange) {
151153
if(FAILED(res)) {
152154
LOG_DEBUGWARNING(L"column.showDetail failed with code "<<res);
153155
}
154-
states|=(showDetail?NVSTATE_EXPANDED:NVSTATE_COLLAPSED);
156+
nvCellStates |= (showDetail ? NvCellState::EXPANDED : NvCellState::COLLAPSED);
155157
}
156158
}
157159
}
@@ -162,7 +164,7 @@ __int64 getCellStates(HWND hwnd, IDispatch* pDispatchRange) {
162164
LOG_DEBUGWARNING(L"range.hasFormula failed with code "<<res);
163165
}
164166
if(hasFormula) {
165-
states|=NVSTATE_HASFORMULA;
167+
nvCellStates |= NvCellState::HASFORMULA;
166168
}
167169
// Expose whether this cell has a dropdown menu for choosing valid values
168170
CComPtr<IDispatch> pDispatchValidation=nullptr;
@@ -179,7 +181,7 @@ __int64 getCellStates(HWND hwnd, IDispatch* pDispatchRange) {
179181
LOG_DEBUGWARNING(L"validation.type failed with code "<<res);
180182
}
181183
if(validationType==xlValidateList) {
182-
states|=NVSTATE_HASPOPUP;
184+
nvCellStates |= NvCellState::HASPOPUP;
183185
}
184186
}
185187
// Expose whether this cell has comments
@@ -189,7 +191,7 @@ __int64 getCellStates(HWND hwnd, IDispatch* pDispatchRange) {
189191
LOG_DEBUGWARNING(L"range.comment failed with code "<<res);
190192
}
191193
if(pDispatchComment) {
192-
states|=NVSTATE_HASCOMMENT;
194+
nvCellStates |= NvCellState::HASCOMMENT;
193195
}
194196
// Expose whether this cell is unlocked for editing
195197
BOOL locked=false;
@@ -210,7 +212,7 @@ __int64 getCellStates(HWND hwnd, IDispatch* pDispatchRange) {
210212
LOG_DEBUGWARNING(L"worksheet.protectcontents failed with code "<<res);
211213
}
212214
if(protectContents) {
213-
states|=NVSTATE_UNLOCKED;
215+
nvCellStates |= NvCellState::UNLOCKED;
214216
}
215217
}
216218
}
@@ -227,7 +229,7 @@ __int64 getCellStates(HWND hwnd, IDispatch* pDispatchRange) {
227229
LOG_DEBUGWARNING(L"hyperlinks.count failed with code "<<res);
228230
}
229231
if(count>0) {
230-
states|=NVSTATE_LINKED;
232+
nvCellStates |= NvCellState::LINKED;
231233
}
232234
}
233235
// Expose whether this cell's content flows outside the cell,
@@ -304,17 +306,17 @@ __int64 getCellStates(HWND hwnd, IDispatch* pDispatchRange) {
304306
LOG_DEBUGWARNING(L"range.text failed with code "<<res);
305307
}
306308
if(text&&text.Length()>0) {
307-
states|=NVSTATE_CROPPED;
309+
nvCellStates |= NvCellState::CROPPED;
308310
}
309311
}
310-
if(!(states&NVSTATE_CROPPED)) {
311-
states|=NVSTATE_OVERFLOWING;
312+
if(!(nvCellStates & NvCellState::CROPPED)) {
313+
nvCellStates |= NvCellState::OVERFLOWING;
312314
}
313315
}
314316
}
315317
}
316318
}
317-
return states;
319+
return nvCellStates;
318320
}
319321

320322
HRESULT getCellInfo(HWND hwnd, IDispatch* pDispatchRange, long cellInfoFlags, EXCEL_CELLINFO* cellInfo) {
@@ -364,7 +366,7 @@ HRESULT getCellInfo(HWND hwnd, IDispatch* pDispatchRange, long cellInfoFlags, EX
364366
}
365367
}
366368
if(cellInfoFlags&NVCELLINFOFLAG_STATES) {
367-
cellInfo->states=getCellStates(hwnd,pDispatchRange);
369+
cellInfo->nvCellStates = getCellStates(hwnd, pDispatchRange);
368370
}
369371
CComPtr<IDispatch> pDispatchMergeArea=nullptr;
370372
if(cellInfoFlags&NVCELLINFOFLAG_COORDS||cellInfoFlags&NVCELLINFOFLAG_OUTLINELEVEL) {

‎nvdaHelper/remote/excel/Constants.h‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
/*
22
This file is a part of the NVDA project.
3-
Copyright 2019-2020 NV Access Limited, Accessolutions, Julien Cochuyt
4-
This program is free software: you can redistribute it and/or modify
5-
it under the terms of the GNU General Public License version 2.0, as published by
6-
the Free Software Foundation.
7-
This program is distributed in the hope that it will be useful,
8-
but WITHOUT ANY WARRANTY; without even the implied warranty of
9-
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
3+
Copyright 2019-2022 NV Access Limited, Accessolutions, Julien Cochuyt
4+
This program is free software: you can redistribute it and/or modify
5+
it under the terms of the GNU General Public License version 2.0, as published by
6+
the Free Software Foundation.
7+
This program is distributed in the hope that it will be useful,
8+
but WITHOUT ANY WARRANTY; without even the implied warranty of
9+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
1010
This license can be found at:
1111
http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
1212
*/
@@ -78,17 +78,25 @@ const long NVCELLINFOFLAG_COMMENTS=0x40;
7878
const long NVCELLINFOFLAG_FORMULA=0x80;
7979
const long NVCELLINFOFLAG_ALL=0xffff;
8080

81-
// NVDA states
82-
const __int64 NVSTATE_EXPANDED=0x100;
83-
const __int64 NVSTATE_COLLAPSED=0x200;
84-
const __int64 NVSTATE_LINKED=0x1000;
85-
const __int64 NVSTATE_HASPOPUP=0x2000;
86-
const __int64 NVSTATE_PROTECTED=0x4000;
87-
const __int64 NVSTATE_HASFORMULA=0x1000000000;
88-
const __int64 NVSTATE_HASCOMMENT=0x2000000000;
89-
const __int64 NVSTATE_CROPPED=0x8000000000;
90-
const __int64 NVSTATE_OVERFLOWING=0x10000000000;
91-
const __int64 NVSTATE_UNLOCKED=0x20000000000;
81+
constexpr std::uint64_t setBit(const unsigned int bitPos) {
82+
return std::uint64_t(1) << bitPos;
83+
}
84+
85+
/*NVDA sell specific states.
86+
These values must match NvCellState enum in source/nvdaObjects/excel.py
87+
*/
88+
enum NvCellState : std::uint64_t {
89+
EXPANDED = setBit(1),
90+
COLLAPSED = setBit(2),
91+
LINKED = setBit(3),
92+
HASPOPUP = setBit(4),
93+
PROTECTED = setBit(5),
94+
HASFORMULA = setBit(6),
95+
HASCOMMENT = setBit(7),
96+
CROPPED = setBit(8),
97+
OVERFLOWING = setBit(9),
98+
UNLOCKED = setBit(10)
99+
};
92100

93101
// an HRESULT error code randomly given by Excel such as for validation.type when there is no validation on the cell
94102
const HRESULT XLGeneralError=0x800a03ec;

‎source/NVDAObjects/window/excel.py‎

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@
55

66
import abc
77
import ctypes
8+
import enum
9+
from typing import (
10+
Optional, Dict,
11+
)
12+
813
from comtypes import COMError, BSTR
914
import comtypes.automation
1015
import wx
@@ -1059,13 +1064,42 @@ def _get_locationText(self):
10591064
NVCELLINFOFLAG_FORMULA=0x80
10601065
NVCELLINFOFLAG_ALL=0xffff
10611066

1067+
1068+
class NvCellState(enum.IntEnum):
1069+
# These values must match NvCellState in `nvdaHelper/remote/excel/constants.h`
1070+
EXPANDED = 1 << 1,
1071+
COLLAPSED = 1 << 2,
1072+
LINKED = 1 << 3,
1073+
HASPOPUP = 1 << 4,
1074+
PROTECTED = 1 << 5,
1075+
HASFORMULA = 1 << 6,
1076+
HASCOMMENT = 1 << 7,
1077+
CROPPED = 1 << 8,
1078+
OVERFLOWING = 1 << 9,
1079+
UNLOCKED = 1 << 10,
1080+
1081+
1082+
_nvCellStatesToStates: Dict[NvCellState, controlTypes.State] = {
1083+
NvCellState.EXPANDED: controlTypes.State.EXPANDED,
1084+
NvCellState.COLLAPSED: controlTypes.State.COLLAPSED,
1085+
NvCellState.LINKED: controlTypes.State.LINKED,
1086+
NvCellState.HASPOPUP: controlTypes.State.HASPOPUP,
1087+
NvCellState.PROTECTED: controlTypes.State.PROTECTED,
1088+
NvCellState.HASFORMULA: controlTypes.State.HASFORMULA,
1089+
NvCellState.HASCOMMENT: controlTypes.State.HASCOMMENT,
1090+
NvCellState.CROPPED: controlTypes.State.CROPPED,
1091+
NvCellState.OVERFLOWING: controlTypes.State.OVERFLOWING,
1092+
NvCellState.UNLOCKED: controlTypes.State.UNLOCKED,
1093+
}
1094+
1095+
10621096
class ExcelCellInfo(ctypes.Structure):
10631097
_fields_=[
10641098
('text',comtypes.BSTR),
10651099
('address',comtypes.BSTR),
10661100
('inputTitle',comtypes.BSTR),
10671101
('inputMessage',comtypes.BSTR),
1068-
('states',ctypes.c_longlong),
1102+
('nvCellStates', ctypes.c_longlong), # bitwise OR of the NvCellState enum values.
10691103
('rowNumber',ctypes.c_long),
10701104
('rowSpan',ctypes.c_long),
10711105
('columnNumber',ctypes.c_long),
@@ -1075,6 +1109,7 @@ class ExcelCellInfo(ctypes.Structure):
10751109
('formula',comtypes.BSTR),
10761110
]
10771111

1112+
10781113
class ExcelCellInfoQuickNavItem(browseMode.QuickNavItem):
10791114

10801115
def __init__( self , parentIterator, cellInfo):
@@ -1181,7 +1216,10 @@ def collectionFromWorksheet(self,worksheetObject):
11811216

11821217
class ExcelCell(ExcelBase):
11831218

1184-
def _get_excelCellInfo(self):
1219+
excelCellInfo: Optional[ExcelCellInfo]
1220+
"""Type info for auto property: _get_excelCellInfo"""
1221+
1222+
def _get_excelCellInfo(self) -> Optional[ExcelCellInfo]:
11851223
if not self.appModule.helperLocalBindingHandle:
11861224
return None
11871225
ci=ExcelCellInfo()
@@ -1382,10 +1420,14 @@ def _get_states(self):
13821420
cellInfo=self.excelCellInfo
13831421
if not cellInfo:
13841422
return states
1385-
stateBits=cellInfo.states
1386-
for state in controlTypes.State:
1387-
if stateBits & state.value:
1388-
states.add(state)
1423+
nvCellStates = cellInfo.nvCellStates
1424+
1425+
for possibleCellState in NvCellState:
1426+
if nvCellStates & possibleCellState.value:
1427+
states.add(
1428+
# intentionally use indexing operator so an error is raised for a missing key
1429+
_nvCellStatesToStates[possibleCellState]
1430+
)
13891431
return states
13901432

13911433
def event_typedCharacter(self,ch):

‎tests/unit/test_excel.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# A part of NonVisual Desktop Access (NVDA)
2+
# This file is covered by the GNU General Public License.
3+
# See the file COPYING for more details.
4+
# Copyright (C) 2022 NV Access Limited
5+
6+
"""Unit tests for the excel module.
7+
"""
8+
9+
import unittest
10+
import NVDAObjects.window.excel as excel
11+
12+
13+
class TestCellStates(unittest.TestCase):
14+
def test_cellStateMapsToState(self):
15+
"""All Excel Cell info states should map to a controlTypes.State
16+
"""
17+
for cellState in excel.NvCellState:
18+
excel._nvCellStatesToStates[cellState] # throws if cellState is missing

‎user_docs/en/changes.t2t‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,8 @@ This can dramatically decrease build times on multi core systems. (#13226, #1337
156156
- ``UIAHandler.UIAControlTypesToNVDARoles``
157157
-
158158
-
159+
- Excel cell state constants (``NVSTATE_*``) are now values in the ``NvCellState`` enum, mirrored in the ``NvCellState`` enum in ``NVDAObjects/window/excel.py`` and mapped to ``controlTypes.State`` via _nvCellStatesToStates. (#13465)
160+
- ``EXCEL_CELLINFO`` struct member ``state`` is now ``nvCellStates``.
159161
-
160162

161163

0 commit comments

Comments
 (0)