Skip to content

Correctly announce selection changes in inline cel edit control in Microsoft Excel#16473

Merged
seanbudd merged 7 commits into
masterfrom
fixExcelTextSelection
May 8, 2024
Merged

Correctly announce selection changes in inline cel edit control in Microsoft Excel#16473
seanbudd merged 7 commits into
masterfrom
fixExcelTextSelection

Conversation

@michaelDCurran

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #15843

Summary of the issue:

When editing a cell in Microsoft Excel with inline cell editing enabled, NvDA does not announce text selection changes.
This is because in order to support inline cell editing, NvDA needs to redirect focus to the real cell edit control - Excel fires MSAA focus on a broken control. However, caret events do not know that focus was redirected and only go to the last queued focus object, which is still the broken control.
Note that this already works when NVDA uses UIA to access MS Excel (as the UIA control is not broken).

Description of user facing changes

NVDA announces selection changes in the inline cel edit control in Microsoft Excel.

Description of development approach

Rather than redirecting focus events to an obj's focusRedirect property in executeEvent, do it earlier in queueEvent, so that lastQueuedFocusObject is the possibbly redirected focus and therefore other code paths in NVDA check against what the focus actually ends up being.

Testing strategy:

  • Open Microsoft Excel.
  • Ensure NVDA is not using UIA to access Microsoft Excel.
  • Arrow to a cell.
  • Press f2 to edit.
  • Type some text.
  • Use shift+arrow keys to select some text.
  • Verify that NVDA announces the text being selected.

Known issues with pull request:

This change is broader than the specific scenario being fixed, though it feels logically like this is what it should have been like in the first place.
But to be safe, this is going to master first, and if any weirdness with events associated with the focus is seen, then this can be reverted and a more specific fix can be made. Such as in IAccessiblehandler's processGenericEvent, specifically redirect caret events to to focusRedirect on EXCEL6 objects.

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.

…sRedirect property if set, rather than doing this later in executeEvent.
@michaelDCurran michaelDCurran requested a review from a team as a code owner May 1, 2024 02:38
Comment thread user_docs/en/changes.t2t Outdated
Comment thread user_docs/en/changes.t2t Outdated
michaelDCurran and others added 2 commits May 1, 2024 13:17
Co-authored-by: Sascha Cowley <16543535+SaschaCowley@users.noreply.github.com>
Comment thread user_docs/en/changes.t2t Outdated
Co-authored-by: Sean Budd <sean@nvaccess.org>
@seanbudd seanbudd added this to the 2024.3 milestone May 1, 2024
@seanbudd seanbudd added queued for merge conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. labels May 6, 2024
@seanbudd seanbudd merged commit 3dd693f into master May 8, 2024
@seanbudd seanbudd deleted the fixExcelTextSelection branch May 8, 2024 02:22
@Emil-18

Emil-18 commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

@michaelDCurran I tested with NVDA 2024.3, and the issue is still present. I then checked the code of the latest alpha, and focusRedirect is still handled in executeEvent rather then queueEvent?

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. queued for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: when selecting text in a cell after hitting f2, NVDA does not read what is selected when shift+arrows are used.

4 participants