Skip to content

Excel with UIA: announce merged cells#12843

Merged
seanbudd merged 6 commits into
nvaccess:masterfrom
LeonarddeR:excelMergedUIA
Jun 20, 2022
Merged

Excel with UIA: announce merged cells#12843
seanbudd merged 6 commits into
nvaccess:masterfrom
LeonarddeR:excelMergedUIA

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Sep 14, 2021

Copy link
Copy Markdown
Collaborator

Link to issue number:

None

Summary of the issue:

When UIA is enabled for Excel, merged cells were not mentioned as such. Instead, only the first coordinates of the merged range were reported.

Description of how this pull request fixes the issue:

There is no way to fetch the last cell in the range from Excel itself. The UIA implementation in Excel also refuses to provide real row and column numbers. Therefore we have to convert the alphabetical column representation to a column number, then correct the column number for column span and convert it back to alphabetical representation.

Testing strategy:

Tested several merged cells on an empty Excel sheet.

Known issues with pull request:

  1. Excel seems to expose only the visible part of the sheet to UIA. Therefore, if a merged cell spans more rows or columns than what's visible, the announcements are likely incorrect. Note that this also applies to selection.
  2. Initiating selection from a merged cell is broken, because the selection pattern reveals a null pointer as the first selected value. This is definitely a bug in Excel.

Change log entries:

Changes

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual testing.
  • API is compatible with existing addons.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers

@LeonarddeR LeonarddeR requested a review from a team as a code owner September 14, 2021 06:42
@seanbudd seanbudd added this to the 2021.3 milestone Oct 15, 2021
@feerrenrut

Copy link
Copy Markdown
Contributor

@seanbudd Could you elaborate on why this must go into the 2021.3 release, there doesn't seem to be a justification provided.

@cary-rowen

Copy link
Copy Markdown
Contributor

I am a frequent user of excel. I hope that this PR can be merged. Is there any problem that prevents this PR?

@seanbudd seanbudd self-requested a review October 18, 2021 05:20
@seanbudd seanbudd removed this from the 2021.3 milestone Oct 18, 2021

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @LeonarddeR - do you know if MS is aware of the bug(s)?

Comment thread source/NVDAObjects/UIA/excel.py Outdated
Comment thread source/NVDAObjects/UIA/excel.py Outdated
for i in modGenerator(n)
)[::-1]

def _getNumberRepresentationForColumn(self, column):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could this get a unit test with a couple of examples?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it appears that unittests cannot import from NVDAObjects.UIA.excel

======================================================================
ERROR: Failure: AttributeError ('NoneType' object has no attribute 'registerUIAProperty')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\sean\projects\nvda\.venv\lib\site-packages\nose\failure.py", line 39, in runTest       
    raise self.exc_val.with_traceback(self.tb)
  File "C:\Users\sean\projects\nvda\.venv\lib\site-packages\nose\loader.py", line 418, in loadTestsFromName
    addr.filename, addr.module)
  File "C:\Users\sean\projects\nvda\.venv\lib\site-packages\nose\importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "C:\Users\sean\projects\nvda\.venv\lib\site-packages\nose\importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "C:\Users\sean\AppData\Local\Programs\Python\Python37-32\lib\imp.py", line 234, in load_module   
    return load_source(name, filename, file)
  File "C:\Users\sean\AppData\Local\Programs\Python\Python37-32\lib\imp.py", line 171, in load_source   
    module = _load(spec)
  File "<frozen importlib._bootstrap>", line 696, in _load
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "C:\Users\sean\projects\nvda\tests\unit\test_excel.py", line 11, in <module>
    from NVDAObjects.UIA.excel import ExcelCell
  File "C:\Users\sean\projects\nvda\source\NVDAObjects\UIA\__init__.py", line 928, in <module>
    class UIA(Window):
  File "C:\Users\sean\projects\nvda\source\NVDAObjects\UIA\__init__.py", line 929, in UIA
    _UIACustomProps = UIAHandler.customProps.CustomPropertiesCommon.get()
  File "C:\Users\sean\projects\nvda\source\UIAHandler\customProps.py", line 75, in get
    cls._instance = cls()
  File "C:\Users\sean\projects\nvda\source\UIAHandler\customProps.py", line 83, in __init__
    uiaType=UIAutomationType.INT,
  File "<string>", line 6, in __init__
  File "C:\Users\sean\projects\nvda\source\UIAHandler\customProps.py", line 54, in __post_init__        
    self.id = NVDAHelper.localLib.registerUIAProperty(
AttributeError: 'NoneType' object has no attribute 'registerUIAProperty'

Comment thread source/NVDAObjects/UIA/excel.py Outdated
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Do you know if MS is aware of the bug(s)?

Nope. May be @michaelDCurran knows. I believe there's no such thing as a presentational row or column number like the ones implemented for IAccessible2. That would certainly help.

@feerrenrut feerrenrut added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Mar 18, 2022
@seanbudd seanbudd removed the request for review from michaelDCurran June 14, 2022 04:18
@seanbudd seanbudd merged commit 0976a98 into nvaccess:master Jun 20, 2022
@nvaccessAuto nvaccessAuto added this to the 2022.3 milestone Jun 20, 2022
@LeonarddeR LeonarddeR deleted the excelMergedUIA branch August 23, 2025 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants