Address crash in Excel 2007#9456
Merged
Merged
Conversation
…nge objects between NvDA and nvdaHelper inproc code, rather than letting Windows do it, as the previous implementation was failing on Office 2007 because CoInitialize was never called on the RPC worker thread.
feerrenrut
approved these changes
Apr 5, 2019
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 #9431
Summary of the issue:
With the merging of PR #9257 which greatly increased performance in Excel, Excel 2007 began to crash on start-up for some users. And for others, NVDA would simply fail to report whether a cell had a formular or other related features.
In order to increase performance, NVDA began collecting all required cell information on one corss-process call, rather than via individusl cross-process calls. In order to communicate the correct cell range to fetch info for, the IDispatch pointer for that range was marshalled via RPC into Excel's process. Although this works for Excel 2010 and up, it seems that in Excel 2007, this is causing exceptions in the RPC infrastructure, specifically the error stating that CoInitialize had not been called on the RPC worker thread executing the in-process implementation of our excel_getCellInfos rpc function.
Description of how this pull request fixes the issue:
This PR stops relying on Windows to try and marshal the Excel range object, and instead the range's address is passed to the rpc function as a string. Then in Excel's main thread, a reference to Excel's object model is retreaved and a range object is created using the passed in address.
This solution works for Excel 2007 up to Excel 365 and does not seem to lose any performance gain.
Testing performed:
Opened Excel 2007 and focused on a cell. Excel no longer crashes on the machine it used to crash on.
Ensured that NVDA reported "has formular" for cells that contain a fomrular, and "has comment" when a comment is inserted in the cell using NVDA.
These same tests were performed on Excel 365 with the same results.
The original tests from pr #9257 were also performed on Excel 2007 / 365 with no loss in performance or functionality.
Excel 2013 has also been tested with similar results.
Known issues with pull request:
None.
Change log entry:
Bug fixes: