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

Address crash in Excel 2007 #9456

Merged
merged 4 commits into from Apr 7, 2019

Conversation

Projects
None yet
3 participants
@michaelDCurran
Copy link
Contributor

commented Apr 4, 2019

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:

  • NVDA no longer causes Excel 2007 to crash or refuses to report if a cell has a formular. (#9431)

michaelDCurran added some commits Apr 3, 2019

Excel: use LresultFromObject and objectFromLResult to marshal cell ra…
…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.

@michaelDCurran michaelDCurran requested a review from feerrenrut Apr 4, 2019

@michaelDCurran michaelDCurran merged commit e194b89 into rc Apr 7, 2019

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@nvaccessAuto nvaccessAuto added this to the 2019.2 milestone Apr 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.