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

Excel: fix comments/formulas listing (NVDA+f7) when they are in non-contiguous cells #17720

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

CyrilleB79
Copy link
Collaborator

@CyrilleB79 CyrilleB79 commented Feb 21, 2025

Link to issue number:

Fixes #11366

Summary of the issue:

In an Excel sheet, when comments or formulas are located in non-contiguous cells (i.e. most of the time), listing them in the elements dialog (NVDA+f7) fails.
It's because our cpp code only support ranges containing one area.

Description of user facing changes

Excel will no longer fail to list formulas/comments in its elements dialog when

Description of development approach

Added one extra loop to retrieve the content for each area separately.

Testing strategy:

Manual test for comments, formulas in non-contiguous cells.
Tested on:

  • Excel 2021 LTSC (Microsoft® Excel® LTSC MSO (16.0.14332.20824) 64 bits)
  • Excel 2016 (Microsoft® Excel® 2016 MSO (Version 2501 Build 16.0.18429.20132) 32 bits)

If someone wants to test on other office versions (e.g. 365, or older ones) feel free to provide feedback. Thanks.

Known issues with pull request:

None

Additional note

I had this code in a branch for many years now.
Maybe we could work on a more optimized architecture if we can convert the cpp code to handle multiple areas.
However, I am not sure to have time or to be able to dig into this.
Since #11366 has not been addressed for so many years, I made the decision to provide my code as is. If we notice performances issues in the future, we still can think to optimize the architecture trying to retrieve the information for the cells of all areas in one call to the cpp code.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

@CyrilleB79 CyrilleB79 marked this pull request as ready for review February 23, 2025 20:48
@CyrilleB79 CyrilleB79 requested a review from a team as a code owner February 23, 2025 20:48
@seanbudd
Copy link
Member

Pending triage of #11366

Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Code changes look good, pending triage/example to test with

@@ -263,7 +264,7 @@ This is a patch release to fix a bug when saving speech symbol dictionaries.

* Fixed bug where speech symbols dictionaries were not saved and the dialog would not be closed. (#17344)

## 2024.4
e## 2024.4
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
e## 2024.4
## 2024.4

for index in range(numCellsFetched.value):
ci = cellInfos[index]
if not ci.address:
log.debugWarning("cellInfo at index %s has no address" % index)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
log.debugWarning("cellInfo at index %s has no address" % index)
log.debugWarning(f"cellInfo at index {index} has no address")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Excel - Listing the comments in the current sheet fails
2 participants