Fix Excel cell reporting lagging behind #14984
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@jcsteh May be you can have a quick look, since this contains some copy paste from your code in editableText |
|
Change log entries:
Bug fixes:
* When rapidly moving cells in Excel, NVDA is now less likely to report the wrong cell or selection. (#14983)
Suggest "When rapidly moving through cells ..."
|
|
@LeonarddeR, do you think this is likely to fix either #12200 or #12108? |
|
Yes, I think it is very likely that these issues will also be fixed by this pr as I can easily clarify these issues now. |
See test results for failed build of commit d310caeb8c |
|
Hi @LeonarddeR |
See test results for failed build of commit 4c163cf88d |
6 tasks
seanbudd
pushed a commit
that referenced
this pull request
Jul 9, 2023
Fixes #15091 Summary of the issue: Pr #14984 broke Excel cell formatting reporting in such a way that it would always report all formatting, regardless of whether the formatting changed between cells. Description of user facing changes Ensure that cell formatting isn't repeated needlessly. Description of development approach When getting the selection in script_changeSelection, override the parent of the selection with self. This ensures that the format field cache on the work sheet will persist when moving through cells.
Merged
6 tasks
seanbudd
pushed a commit
that referenced
this pull request
Jul 17, 2023
…or passthrough on init (#15136) Fixes #15131 Fixes #10612 Summary of the issue: Recent changes to Excel support caused an issue where selection in the Excel cell edit control didn't work. When fixing that, I discovered another issue where initializing the Excel tree interceptor set the passThrough auto property to True, resulting into vision and braille focus set to the previous focus object. Description of user facing changes Selection works again in cell edit controls When leaving a cell edit control, the name of the sheet is announced again, similar when coming from the formula bar. When an excel cell gets focus from outside, focus is no longer needlessly set to the previous focus object. Description of development approach This is another case of an issue that was obfuscated before #14984. There is a workaround in the Excel appModule to fallback on the UIA edit control if the edit control that gets focus when pressing f2 reports as unknown. However, the parent of the UIA control was set to the excel cell, not to the parent of the Excel6 window. This means that the changeSelection script still propagated, i.e. for every selection change related gesture in the cell edit control, NVDA would check whether a new cell was focused. I changed the app module to set the parent of the UIA cell edit to the parent of the Excel6 window. This has the side effect of the sheet name being reported after leaving the edit control because the sheet receives focus again. This is however consistent with behavior prior to the app module workaround was applied, i.e. the time when we relied on the displayText to report the contents. After I applied this change, I discovered that leaving the edit cell raised a COMError on an UIA object. This turned out to be an issue like this: api.setFocusObject was called for the new focus object (i.e. the worksheet) A new tree interceptor was created for the work sheet In the init method of the tree interceptor, self.passThrough was set, which is an auto property calling _set_passThrough on the tree interceptor. _set_passThrough called braille.handler.handleGainFocus and vision.handler.handleGainFocus for the current focus object, which was still the previous focus object (i.e. api.setFocusObject was not yet updated to the new focus). a COMError was raised because the UIA cell edit element was no longer available. I fixed this by setting _passThrough instead, which sets the internal _passThrough boolean without applying the logic that updates focus for vision and braille, which is really not necessary and even harmful to do in init of a tree interceptor. I therefore applied the same change to another incarnation of this issue in the MSHTML virtual buffer.
5 tasks
seanbudd
pushed a commit
that referenced
this pull request
Nov 22, 2023
…5809) Closes #15805 Summary of the issue: In older versions of NVDA, NVDA was providing a speech feedback of the landing cell when using Word native commands for table navigation to jump to first or last cell of row or column: alt+home, alt+end, alt+pgUp and alt+pgDown. From NVDA 2023.2 onwards, this speech feedback is erroneous and reports the origin cell instead of the landing one. This is because the script for this commands was not robust and was taking advantage of existing delays in NVDA processing. in NVDA 2023.2 these delays have been reduced what uncovered this bug. Description of user facing changes The landing cell will be reported correctly again when using native Word table navigation to jump to first or last cell of row or column. Description of development approach Strategy inspired from #14984: send the gesture and check if the cursor location has changed before reporting its new position. If the cursor does not move, e.g. because you call a command to jumpt to first cell while you are already on the first cell, the current cell is reported after a maximum delay set to 0.15 seconds (same value as some other polling scripts).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fixes #14983
Fixes #12200
Fixes #12108?
Summary of the issue:
Moving rapidly in Excel can result in lagging one cell behind when reporting focus.
Description of user facing changes
Rapid movement in Excel will now always report the last cell.
Description of development approach
First and foremost, NVDA was relying on the current focus object to cache the current selection in Excel rather than getting the current selection before executing the gesture. This resulted in the one behind behavior. There was logic to check for selection changes, but it detected a change based on the wrong old selection.
I also revamped the logic a bit based on #14708, i.e. perform three attempts before doing a time.sleep.
Testing strategy:
Tested the str from #14983
Known issues with pull request:
None known
Change log entries:
Bug fixes:
Code Review Checklist: