Massively improve performance navigating Excel spreadsheets containing comments or dropdowns #8019
Conversation
… that can pass before the same script executed is not classed as the script repeating) via a 'repeatTimeout' attribute on the script. If not provided scripts default to 0.5 seconds as was previously the case.
…ls, such as for focus events and changing selection. This massively speeds up performance when navigating a spreadsheet with comments or dropdowns.
source/scriptHandler.py
Outdated
| @@ -184,6 +186,14 @@ def executeScript(script,gesture): | |||
| _lastScriptCount=0 | |||
| _lastScriptRef=scriptRef | |||
| _lastScriptTime=scriptTime | |||
| scriptRepeatTimeout=getattr(scriptFunc,'repeatTimeout',0.5) | |||
| print "Script set repeatTimeout of %s"%scriptRepeatTimeout | |||
|
@derekriemer: I addressed your review action. |
source/scriptHandler.py
Outdated
| @@ -184,6 +186,13 @@ def executeScript(script,gesture): | |||
| _lastScriptCount=0 | |||
| _lastScriptRef=scriptRef | |||
| _lastScriptTime=scriptTime | |||
| scriptRepeatTimeout=getattr(scriptFunc,'repeatTimeout',0.5) | |||
There was a problem hiding this comment.
Since 0.5 is used in 3 places, it might makes sense to put this in a _defaultScriptRepeatSeconds variable.
source/NVDAObjects/window/excel.py
Outdated
| if oldSelection.parent==newSelection.parent: | ||
| # Detect if this script is being repeated a lot (E.g. held down) | ||
| repeated=scriptHandler.getLastScriptRepeatCount()>2 | ||
| if repeated and scriptHandler.isScriptWaiting(): |
There was a problem hiding this comment.
Previously this was done in the while loop until the timeout, is this moved out since we expect this script to get called again? How do we know that this script will be called again?
…It was flawed and didn't actually work.
|
In the end I realised that the changes to scriptHandler were flawed as one of the checks for lastScriptRepeatTimeout were missing and still hardcoded to 0.5. Thus all my other testing was incorrect. Re-testing things seem to be okay without it. Let's see how well it goes in Next for a while. |
|
This causes other lags in Excel and therefore is not a suitable solution to the original issue. Removed from next and closing for now. Will try a different direction. |
Also simplify Excel's changeSelection script and reduce lag a little more.
michaelDCurran
dismissed stale reviews from derekriemer and feerrenrut
via
235c924
|
what's the difference between this PR and #9257? |
|
This was replaced by #9257. |
Link to issue number:
Closes #7348
Summary of the issue:
When an Excel spreadsheet contains one or more comments or data validation dropdowns, most if not all object model calls on cell ranges cause the screen to be redrawn which results in a huge slowdown when moving between cells with NVDA.
This is made even worse if the window is maximized as more screen realestate must be refreshed for each call.
In short, if a spreadsheet contains comments or data validation dropdowns, it is almost impossible to navigate the spreadsheet.
Description of how this pull request fixes the issue:
For both focus events and changeSelection scripts on Excel spreadsheets, NVDA now disables Excel's screen updating for the duration of the event/script. The changeSelection script has also been rewritten slightly to ensure the best performance when moving by one or two cells, but at the same time, not cause a major lag when quickly moving through many.
Testing performed:
In Excel 2016:
Earlier versions of Excel should be also tested, though I expect any added calls are certainly available in Excel 2007 or higher.
Known issues with pull request:
This is really just a temporary solution. We should very soon consider switching to using UIA in Excel 2016. It would be also nice if Microsoft were able to address the underlying object model bug.
Change log entry:
Bug fixes: